Page MenuHomePhabricator

Add SimpleDialog component to Codex
Open, In Progress, MediumPublic

Description

Existing components

MediaWiki community:

External libraries:

Wikimedia Design Style Guide:

Figma spec sheet:

Dialogs use cases

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

I wrote a sketch of a dialog component, before abandoning it and working on other things: https://gerrit.wikimedia.org/r/c/wvui/+/699788 . It's very incomplete, but here are some of the things I had in mind while writing this.

How to open/close the dialog: the open/closed state of the dialog can be managed by the dialog, or by the dialog's parent component, or collaboratively by both. The main options are:

  • Dialog manages it, exposes an open method, closes itself when appropriate. Calling code looks like <wvui-dialog ref="dialog">...</wvui-dialog> <wvui-button @click="$refs.dialog.open()">Open dialog</wvui-button>
  • Parent manages it through a prop, listens to a close event. This is what media search does. Calling code looks like <wvui-dialog :active="isDialogActive" @close="isDialogActive = false">...</wvui-dialog> <wvui-button @click="isDialogActive = true">Open dialog</wvui-button>
  • Use v-model to do basically the above. This is what CX does, but inconsistently (some callers use v-model, some use the prop approach). Calling code would look like <wvui-dialog v-model="isDialogActive">...</wvui-dialog> <wvui-button @click="isDialogActive = true">Open dialog</wvui-button>

I wrote my code using the method approach, but on reflection I think I prefer the v-model approach.

Slot use: I think the main slot should be the body of the dialog, with optional named slots used for the footer and the title (which is part of the header). Needing a primary action in the header is a common enough use case that I think it should be done through props that configure the behavior rather than making everyone put the same thing in the header slot. I don't think this part of the header (+ the close icon) should be overridable through a slot.

Desktop vs mobile mode: The MediaSearch code that I copied detects whether it's in desktop or mobile mode by looking at the mw global, but we can't do that in WVUI. Desktop vs mobile controls whether the primary action appears in the header or the footer, which is a relatively major change and not easily controlled through CSS. Maybe we render it twice and use CSS to hide one or the other, based on the viewport width? Perhaps we can do something similar for fullscreen too.

Focus traps: MediaSearch has a single "landing" div with tabindex=0 at the top of the dialog. OOUI has similar divs called "focus traps" both at the top and bottom, which are designed to prevent the focus from leaving the dialog when the user tabs past the last thing in the dialog. When one of the focus traps is focused, an event handler moves the focus to the first/last focusable element in the dialog. Since finding the first/last focusable element is tricky, I think we might be better off setting the initial focus on the "landing" div (what OOUI calls the top focus trap) and making it loop around such that if the user tabs to the bottom focus trap, we focus the top focus trap (and maybe vice versa, if we can avoid infinite event loops).

Overlays: This isn't implemented in the WIP patch, but I did think about it. Components that use certain kinds of absolutely positioned things that extend outward from them, like the Dropdown menu or the suggestions list in TypeaheadSearch, run into trouble in a dialog. The absolutely positioned menu may try to extend beyond the boundaries of the dialog, but it won't be able to. This is especially noticeable if the dropdown is near the bottom of the dialog. To address this issue, OOUI dialogs have an overlay div that covers the entire screen (like the div MediaSearch calls "overlay" and my code calls "shadow", but this one comes after the frame and doesn't block clicks), and dropdowns in dialogs place their menus in there rather than inline, allowing them to visually break out of the dialog.

Screenshot from 2021-06-17 17-07-55.png (405×723 px, 29 KB)
Screenshot from 2021-06-17 17-08-25.png (550×728 px, 39 KB)
Dropdown is cut off without overlayDropdown extends outside of dialog with overlay

In OOUI, passing this information down correctly is annoying: code in dialogs has to pass { $overlay: this.$overlay } to the constructors of child widgets, and those widgets have to pass it down to their children, etc. One missing link in the chain breaks it, and causes the cut-off behavior. In Vue, we could do this much more conveniently using provide/inject. Dialogs would provide their overlay div as 'overlay', and components like Dropdown that use absolute positioning would use inject: [ 'overlay' ], which gets the overlay value from the nearest ancestor component that provides it. See this sandbox for a code sketch.

Caveats:

  • The OOUI screenshots show the dropdown shortening itself and making itself scrollable a few pixels short of where it would get cut off at the edge of the dialog. This is because of the ClippableElement functionality in OOUI. We haven't implemented anything similar to that in WVUI yet, so WVUI dropdowns wouldn't do this and would truly get cut off (the user would have to scroll the dialog, not the menu itself, to reach the menu items at the bottom).
  • A menu that is located in an overlay rather than absolutely positioned inside its parent component will need to find out where its parent is and actively position itself in order to appear in the right place. In OOUI this is implemented in FloatableElement; we haven't implemented anything similar in WVUI yet, so we would need to do that as well for overlays to be useful.
  • The inject usage isn't as simple as inject: [ 'overlay' ] in real life, at least not in Vue 2. That's because the composition API plugin's provide() isn't compatible with the built-in inject functionality, so the composition API needs to be used on the inject side as well. I imagine this might work better in Vue 3, but I haven't tested that yet.

Random other things about my patch:

  • I don't know why I put the footer out of the frame in my code. That doesn't make sense, the footer should be a child of the frame
  • There's a fullscreen boolean in the data, but it doesn't do much yet, and it should probably be a prop instead, or be done with CSS based on the viewport width
  • I made primaryActionType, primaryActionLabel and primaryAction separate props, but maybe this should be one prop that takes a typed Action object with those three things in it?
Jdlrobson added a subscriber: Jdlrobson.

My team would like to be closely involved in the implementation of this component, as overlays are pretty much integral to the mobile experience as everything there is an overlay / dialog.

In Mobile, I expect we will need to eventually render a dialog using vue-router to manage state.
I wrote a proof of concept a while back.

Regarding fullscreen, I'd suggest making an explicit property for dialogs that should go full screen at lower resolutions. On mobile, the fact they go full screen on tablet/desktop was not a design decision - just an artifact of building mobile first.

Jdlrobson updated the task description. (Show Details)

Change 707650 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[wvui@master] POC: Dialog (simple and complex)

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

How to open/close the dialog: the open/closed state of the dialog can be managed by the dialog, or by the dialog's parent component, or collaboratively by both. The main options are:

  • Dialog manages it, exposes an open method, closes itself when appropriate. Calling code looks like <wvui-dialog ref="dialog">...</wvui-dialog> <wvui-button @click="$refs.dialog.open()">Open dialog</wvui-button>
  • Parent manages it through a prop, listens to a close event. This is what media search does. Calling code looks like <wvui-dialog :active="isDialogActive" @close="isDialogActive = false">...</wvui-dialog> <wvui-button @click="isDialogActive = true">Open dialog</wvui-button>
  • Use v-model to do basically the above. This is what CX does, but inconsistently (some callers use v-model, some use the prop approach). Calling code would look like <wvui-dialog v-model="isDialogActive">...</wvui-dialog> <wvui-button @click="isDialogActive = true">Open dialog</wvui-button>

Dialog state management can't be strictly local to the codebase that the dialog is defined in. If it is, then we'll continue to run into issues like T189569: Avoid contradicting messages between Getting Started and Visual editor for new users. That issue was fixed temporarily – it'll be in production in five years… 😉 – at the cost of implementing a signal in VE for other codebases to trigger (see T235812). Please account for cross-codebase dialog open/close signalling in your design lest our codebases be littered with many more such temporary fixes.

Regarding patch: I was curious to explore the differences between the complex dialog on desktop and mobile and see if the same component could serve them both (which it seems it could).

@phuedx I agree on the comment about dialog state management, but I also think that for this very reason it shouldn't belong in the component library. I would expect this to be a separate library inside MediaWiki core. e.g. maybe it's Vue-router.

The component should be dumb and agnostic to any concept of being open and retain no knowledge of other dialogs. Perhaps that should be split into a separate bug ?

How to open/close the dialog: the open/closed state of the dialog can be managed by the dialog, or by the dialog's parent component, or collaboratively by both. The main options are:

  • Dialog manages it, exposes an open method, closes itself when appropriate. Calling code looks like <wvui-dialog ref="dialog">...</wvui-dialog> <wvui-button @click="$refs.dialog.open()">Open dialog</wvui-button>
  • Parent manages it through a prop, listens to a close event. This is what media search does. Calling code looks like <wvui-dialog :active="isDialogActive" @close="isDialogActive = false">...</wvui-dialog> <wvui-button @click="isDialogActive = true">Open dialog</wvui-button>
  • Use v-model to do basically the above. This is what CX does, but inconsistently (some callers use v-model, some use the prop approach). Calling code would look like <wvui-dialog v-model="isDialogActive">...</wvui-dialog> <wvui-button @click="isDialogActive = true">Open dialog</wvui-button>

I wrote my code using the method approach, but on reflection I think I prefer the v-model approach.

Another thing to consider is cases where state changing behavior should be built into a component for usability and accessibility reasons. With Dialogs for example, pressing ESC should close the dialog, and I'm not sure how that would work with the v-model option. I also think regardless of the approach taken, it feels important to try to find an approach that can be used consistently across DS components with states. Off the top of my head, Popups is another component where there are multiple events that could impact the state (i.e. pressing ESC or clicking out side of the popup box).

Change 707650 abandoned by Jdlrobson:

[wvui@master] POC: Dialog (simple and complex)

Reason:

Not looking for review right now,

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

In GlobalWatchlist we currently use the dialog created by OO.ui.confirm to interact with the viewer - that method returns a promise for which button was clicked (resolved to false if cancel, true if OK) - I would hope that any WVUI dialog component has a similar way to interact with it where it doesn't need to be manually included in the template and told when to display or not, but rather have a public api like OO.ui.confirm does.

Volker_E renamed this task from Add a basic Dialog component to WVUI to Add a basic Dialog component to Codex.Oct 11 2021, 5:11 PM
Volker_E edited projects, added Codex; removed WVUI.
bmartinezcalvo changed the task status from Open to In Progress.Apr 6 2022, 3:32 PM
bmartinezcalvo renamed this task from Add a basic Dialog component to Codex to Add SimpleDialog component to Codex.Apr 8 2022, 12:01 PM
bmartinezcalvo updated the task description. (Show Details)

Hi all,

I've created the Figma spec sheet for the SimpleDialog component, adding the following variants:

  • With and without close action
  • With text in the dialog content
  • With text input in the dialog content
  • With different buttons combination and also with horizontal/stacked buttons (full width buttons for stack variant)

I've updated the task title from Basic Dialog to SimpleDialog since we will have simple and complex dialog variants and I guess we will cover only the simple one in this task.

Here you can view the Figma spec sheet.

NOTE: Do we want to define a min and max width for the dialog? Will it be aligned to the columns of the grid or will have fixed width?

I move the task to Ready for development and I leave the task free to be assigned by whoever starts with its development. If there is a use case missing or something needs to be updated, let me know so I can reassign to me the task.

STH triaged this task as Medium priority.May 1 2022, 1:19 PM

Change 789708 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] [WIP] Dialog component

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

Chiming in from the abstract team! We currently use a dialog that looks like this:

Screen Shot 2022-05-16 at 1.43.37 PM.png (390×622 px, 37 KB)

So it has three points of interaction: two buttons, one that roughly is CONFIRM and one that roughly is CANCEL and an additional icon that, on click, also triggers a CANCEL action

Thanks @JKieserman, this is a helpful example! This is mostly in line with what we've started building: an icon-only "close" button, a primary action that can either be progressive or destructive, and a default action.

The only difference I see between this example and the design spec is the order of the buttons when vertically stacked: in the design spec, the primary action is first (on top). @bmartinezcalvo Do you think the order of the buttons should be configurable, or should the primary action always appear on top?

Chiming in from the abstract team! We currently use a dialog that looks like this:

Screen Shot 2022-05-16 at 1.43.37 PM.png (390×622 px, 37 KB)

So it has three points of interaction: two buttons, one that roughly is CONFIRM and one that roughly is CANCEL and an additional icon that, on click, also triggers a CANCEL action

@JKieserman with the new SimpleDialog component we could cover this use case since we can use both primary-progressive and primary-destructive buttons as primary actions (view these use cases in the spec sheet).

The only difference I see between this example and the design spec is the order of the buttons when vertically stacked: in the design spec, the primary action is first (on top). @bmartinezcalvo Do you think the order of the buttons should be configurable, or should the primary action always appear on top?

As @AnneT comments, the only difference with our new SimpleDialog component is that primary action will always be on top when the buttons are in vertical.

Captura de pantalla 2022-05-17 a las 11.45.14.png (930×1 px, 341 KB)

But this dialog version with vertical buttons should only be used in cases where the button text is too long. So we should always use the version with horizontal buttons, unless the button text is too long, which we should avoid in all buttons to improve the UX Writing experience (review this component recommendation).

Captura de pantalla 2022-05-17 a las 11.51.45.png (668×2 px, 482 KB)

@bmartinezcalvo Another question: The OOUI dialogs scale in and out slightly (see demos) for a more intuitive transition. Is this something we should consider implementing in the Codex version? We could start with the same transitions that are used in OOUI and refine from there.

@bmartinezcalvo Another question: The OOUI dialogs scale in and out slightly (see demos) for a more intuitive transition. Is this something we should consider implementing in the Codex version? We could start with the same transitions that are used in OOUI and refine from there.

@AnneT what if we implement the same behavior as in message component?

  • Fade in: ease (system)
  • Fade out when user closes the dialog with one of the buttons or close: ease-out (user)

Captura de pantalla 2022-05-20 a las 15.48.34.png (554×2 px, 439 KB)

@AnneT about the width of the SimpleDialog, do we want to implement different sizes as we have in OOUI demo or do we want to have one single SimpleDialog size in Codex? I assume we need a single size for now and we would add new sizes in the future if we need them.

Captura de pantalla 2022-05-20 a las 15.51.21.png (340×1 px, 167 KB)

Captura de pantalla 2022-05-20 a las 15.55.04.png (1×2 px, 1 MB)

Captura de pantalla 2022-05-20 a las 15.54.57.png (1×2 px, 1 MB)

@bmartinezcalvo I agree; let's stick with one size for now and add the option for configurable sizes in the future if we need to.

I guess we're technically offering two sizes in the initial implementation: fullscreen or not.