Prevent Missing articles. Fix #333

[sdumetz]

I added a proper error message in the article editor in case of 404 errors.

I set the Article.URI field to read-only.

Then I switched the fields to use <sv-property-view>because that's what it's there for.

Then I added a "textarea" mode for <sv-property-string>, to keep the styling roughly unchanged. This might come handy later.

Then I noticed the inconstant handling of "disabled" in <sv-property-*>. In particular <sv-property-string> relied on CSS's pointer-events: none for its disabled state and thus prevented easy copy-paste of the article's URI. Copying the URI if useful if someone wants to overwrite it via an external PUT request. So I gathered common code in property views to hopefully limit such discrepancies in the future and added a common disabled property.

I used the aria-disabled accessibility property which seemed semantically great for this task. Adding it as a watched property to ensure updates in the off-chance it would one day get dynamically updated.

I had trouble getting the formatting right with the property views in CVArticleTaskView and removing the absolute positioning on ff-scroll-y fixed it. I checked everywhere (that I knew of) it's used and it works fine with a few minor fixes.

My opinion is that this CSS positioning trick creates more problems than it solves and we should try to get rid of it everywhere we can.

It's a bit of a mess in the end, I hope it isn't too hard to read.

[gjcope]

Thanks for the PR! Some inline comments below.

I added a proper error message in the article editor in case of 404 errors.

I set the Article.URI field to read-only.

Looks great.

Then I switched the fields to use <sv-property-view>because that's what it's there for.

In this specific case yes, but there a number of other instances of this in the code like:

dpo-voyager/source/client/ui/story/AnnotationsTaskView.ts

Line 132 in 5728312

<ff-text-edit name="lead" text=${inProps.lead.value} maxLength=${this._leadLimit} @change=${this.onTextEdit}></ff-text-edit>

where it serves an important purpose to allow sanitizing, enforcing restrictions, etc.

Then I added a "textarea" mode for <sv-property-string>, to keep the styling roughly unchanged. This might come handy later.

This seems a little inconsistent to me but I think I just need more time to consider it.

Then I noticed the inconstant handling of "disabled" in <sv-property-*>. In particular <sv-property-string> relied on CSS's pointer-events: none for its disabled state and thus prevented easy copy-paste of the article's URI. Copying the URI if useful if someone wants to overwrite it via an external PUT request. So I gathered common code in property views to hopefully limit such discrepancies in the future and added a common disabled property.

I used the aria-disabled accessibility property which seemed semantically great for this task. Adding it as a watched property to ensure updates in the off-chance it would one day get dynamically updated.

This is all good. Love the accessibility consideration - just need to test it out a bit.

I had trouble getting the formatting right with the property views in CVArticleTaskView and removing the absolute positioning on ff-scroll-y fixed it. I checked everywhere (that I knew of) it's used and it works fine with a few minor fixes.

My opinion is that this CSS positioning trick creates more problems than it solves and we should try to get rid of it everywhere we can.

This seems potentially problematic but I will do some more testing.

[sdumetz]

Then I switched the fields to use because that's what it's there for.

In this specific case yes, but there a number of other instances of this in the code [...]
where it serves an important purpose to allow sanitizing, enforcing restrictions, etc.

Whenever there is logic that is owned by the component we should definitely keep it this way. The sanitization is a good example of that. I only see a point in doing this if it allows removing some code, otherwise it's just pointlessly moving things around.

Then I added a "textarea" mode for , to keep the styling roughly unchanged. This might come handy later.

This seems a little inconsistent to me but I think I just need more time to consider it.

Not 100% sure about it either. Not a lot of place for it to be used and your example on annotations made me think maybe we ought to sanitize this too and not doing so was an oversight? If so we might have no place at all where this is used and I should just drop it.

[gjcope]

Not 100% sure about it either. Not a lot of place for it to be used and your example on annotations made me think maybe we ought to sanitize this too and not doing so was an oversight? If so we might have no place at all where this is used and I should just drop it.

The goal with sanitizing specifically was to address unsafe situations where we are allowing HTML tags to be entered by the end user and assigned to innerHTML or using unsafeHTML(). So far, that has been on a case-by-case basis as it has been requested which is admittedly inconsistent. I think it's probably fine to support at least <i> and <b> more broadly. Have you seen demand for this?

[sdumetz]

I think it's probably fine to support at least <i> and <b> more broadly. Have you seen demand for this?

Not a lot. It's just that having some "text blocks" that are "text-only" and others that are "sanitized HTML" is inconsistent. But allowing html in every text blocks isn't the only way to go about it, we could also style the HTML-enabled box differently.

[gjcope]

I think the challenge is indicating what html is available (which we don't do now at all). For our use cases, we want to limit user-provided html as much as we can to ensure a more uniform end-user experience. Ex: italics need to be supported most everywhere for scientific names, etc., but links would only make sense in text bodies (not titles).

Feel free to propose something or maybe this specific item gets tackled as a separate issue.

[sdumetz]

I left the lead field and the textarea addition to string properties aside for now.

I'll try to think of a good visual indication for "light HTML" blocks and let you know if it works.