Page MenuHomePhabricator

Field: Support custom HTML in status messages
Closed, ResolvedPublic2 Estimated Story Points

Description

As a developer I want to be able to add html status messages underneath a cdx-field.

Current situation:
It only supports string values as messages.

<cdx-field :status="status" :messages="messages">
...
</cdx-field>

const status = ref( 'warning' );
const messages = computed( () => {
  return {
    warning: `The username was automatically changed from "${ originalName.value }" to "${ inputValue.value }".`
  };
} );

Desirable situation:
We would like to be able to show customized warning messages that might include HTML. Like so (see <kbd>Ctrl<kbd>):

Screenshot 2024-06-27 at 17.30.51.png (224×972 px, 45 KB)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
CCiufo-WMF triaged this task as Medium priority.Jul 16 2024, 4:20 PM
CCiufo-WMF moved this task from Inbox to Up Next on the Design-System-Team board.
CCiufo-WMF renamed this task from Allow html status messages on cdx-field to Field: Support custom HTML in status messages.Aug 23 2024, 7:46 PM
CCiufo-WMF moved this task from Up Next to Needs Refinement on the Design-System-Team board.
CCiufo-WMF set the point value for this task to 2.Sep 9 2024, 5:11 PM

Change #1088300 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] Field: support custom messages with HTML

https://gerrit.wikimedia.org/r/1088300

@lwatson After thinking about this for a bit, I think we should explore an alternate approach here. I gave some suggestions in Slack before I had time to completely consider the implications.

Allowing users to provide HTML markup strings as component props is something we should avoid if possible. Even if the immediate use-case is low risk, using v-html here exposes all users of Codex to potentially dangerous content. It also sets a bad precedent.

In your patch you correctly note the difference between how the Field component (which takes error message content via props) and the Message component (which takes content via slot) work. If we want to allow arbitrary markup to appear in a Codex component, exposing a slot is the "Vue Way" to do this. My recommendation is to keep the current patch up for comparison (I'll keep the -2 there to ensure it is not merged without further discussion), and to open a new patch that pursues a slot-based approach.

What if the Field component exposed a separate <slot> for each potential status? The component could even be written in such a way that an error message prop would be ignored if an #error slot was provided. Then users would have a choice – if all you need is a string error message, then providing the message via props is fine. If custom components or other markup is needed, then use a slot instead.

I think we should explore a slot-based approach here – if that proves untenable, then we can reconsider the initial patch (but security concerns may mean it can't move forward).

For more on Vue security best-practices, see https://vuejs.org/guide/best-practices/security.

@egardner, thanks for the feedback! I'll open a patch to display custom messages in the Field using a slot-based approach.

If the custom markup is always going to come from i18n messages defined in TranslateWiki, then there is a MediaWiki helper called v-i18n-html. This is not part of Codex but it's automatically available to all Vue code inside MW. You can see the code (which has a lot of inline documentation) here: https://github.com/wikimedia/mediawiki/blob/master/resources/src/vue/i18n.js

Here's an approach I think could work:

  • Field component exposes one or more optional slots (maybe one for each status)
  • If the given slot is provided, then that gets used instead of the status message in the status object

The key thing is that v-html should not live in Codex; it should always be called by the user, meaning that using it in a secure way is always the user's responsibility (not Codex's).

Usage could look like this:

<cdx-field>
    <template #error>
        <!-- this is provided by the user and they can determine if this is safe or not -->
        <span v-i18n-html="myMarkupErrorMessage" />
    </template>

    <!-- other field content as usual... -->
</cdx-field>

This may need to be handled as a scoped slot if it needs access to other internal data from Field.

There is some new logic that will be needed to manage whether to show slot content, content from the status object, etc. This will need to be tested, and there may be some other considerations we need to address too.

Change #1088607 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] Field: add custom status message slots

https://gerrit.wikimedia.org/r/1088607

Change #1088607 merged by jenkins-bot:

[design/codex@main] Field: add custom status message slots

https://gerrit.wikimedia.org/r/1088607

Change #1090947 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[mediawiki/core@master] Update Codex from v1.15.0 to v1.16.0

https://gerrit.wikimedia.org/r/1090947

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
http://patchdemo.wmcloud.org/wikis/a2a0917c1b/w/

Change #1090947 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v1.15.0 to v1.16.0

https://gerrit.wikimedia.org/r/1090947

AnneT subscribed.

@DSmit-WMF slots for message content are now available in the Field component! Please let us know if you run into any issues using them.

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

http://patchdemo.wmcloud.org/wikis/a2a0917c1b/w/