Page MenuHomePhabricator

Consider adding suggestion widget to OOUI
Open, Needs TriagePublic

Description

We now have two extensions (MachineVision and GrowthExperiments ) using a Suggestion Widget (see T237585 and T238610 for screenshots). It would be nice to consider upstreaming this so that both projects (and others in the future) can benefit from shared maintenance and development. At the moment we in Growth have copy-pasted (🙀) the code from MachineVision.

Event Timeline

kostajh created this task.Jan 10 2020, 10:25 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 10 2020, 10:25 AM
AnneT updated the task description. (Show Details)
AnneT updated the task description. (Show Details)

That looks basically the same as our CheckboxMultiselectWidget.

There are some issues with the existing code in MachineVision that I ran into when I copied it to GrowthExperiments:

  • It depends on mediawiki.template.mustache+dom, which is in the WikibaseMediaInfo extension (MachineVision already depends on WBMI, but it doesn't make sense for GrowthExperiments to depend on WBMI)
  • The mustache+dom templating engine avoids destructively rerendering embedded OOUI widgets, which is good, but it does destructively rerender nodes created by the template. This is a problem, because it breaks CSS transitions for state changes.
  • Because CSS transitions don't work, the widget uses CSS animations instead. But that causes a bug where the widgets animate (the text moves from left to right) when they're being unhidden or added to the DOM.
  • There is no clean group widget for these, so I had to make one by picking the relevant pieces from ImageWithSuggestionsWidget (which contains a lot of other things as well). Without that, the SuggestionWidgets don't behave correctly, because they rely on a CSS class set on the parent element to hide the outline.
  • These widgets don't extend SelectWidget and OptionWidget, which means you have to reinvent some wheels around selection management and getting the selected values from the group widget for example

Overall I agree with @matmarex that this is functionally a CheckboxMultiselectWidget, just with different styling.

I'd recommend upstreaming this into MW as a MediaWiki widget first, before moving all the way to OOUI.

Restricted Application added a project: Structured-Data-Backlog. · View Herald TranscriptMar 16 2020, 3:33 PM
RHo added a comment.Apr 7 2020, 7:39 PM

There are some issues with the existing code in MachineVision that I ran into when I copied it to GrowthExperiments:

  • It depends on mediawiki.template.mustache+dom, which is in the WikibaseMediaInfo extension (MachineVision already depends on WBMI, but it doesn't make sense for GrowthExperiments to depend on WBMI)
  • The mustache+dom templating engine avoids destructively rerendering embedded OOUI widgets, which is good, but it does destructively rerender nodes created by the template. This is a problem, because it breaks CSS transitions for state changes.
  • Because CSS transitions don't work, the widget uses CSS animations instead. But that causes a bug where the widgets animate (the text moves from left to right) when they're being unhidden or added to the DOM.
  • There is no clean group widget for these, so I had to make one by picking the relevant pieces from ImageWithSuggestionsWidget (which contains a lot of other things as well). Without that, the SuggestionWidgets don't behave correctly, because they rely on a CSS class set on the parent element to hide the outline.
  • These widgets don't extend SelectWidget and OptionWidget, which means you have to reinvent some wheels around selection management and getting the selected values from the group widget for example

Overall I agree with @matmarex that this is functionally a CheckboxMultiselectWidget, just with different styling.

While I agree this is the CheckboxMultiselectWidget with different styling, these differences make a big difference to the user interaction. The styling is both meant to be more more touch-friendly, and also enables more compact presentation of a checkbox group inline:

vs

Also, I am hoping that we can solve how to get the animation when an item is selected (check mark appearing and the text moving to the right) as part of incorporating this into the OOUI library.

cc @Volker_E - per our conversation earlier today, can you provide some notes on how we can go about getting this moved forward? Thanks! :)

RHo added a subscriber: mwilliams.Apr 7 2020, 7:41 PM

@RHo Quick wiring up of OOUI's CheckboxMultiselectWidget to reflect design in your screen:

. Clearly this was a quick test (~30 mins) that doesn't take into account things like animation or perfect relative sizing yet. But I'm convinced that most things design are able to be copied.

In order to get this into OOUI, I'd suggest to a) refactor the current codebase and follow both, @matmarex and @Catrope's advice to use CheckboxMultiselectWidget and b) then decide if it better lives in MediaWiki core, or if it makes sense to have it as part of OOUI. Extensions can use them from both places pretty equally, it's just a matter of how much do we think it is an integral part of the general user library as often recurring pattern.

AnneT added a comment.Apr 8 2020, 1:23 PM

I recently refactored the suggestion component in Machine Vision to use CheckboxMultiselectWidget, in case that's helpful. See SuggestionsWidget in this patch, or check out the current code (JS | LESS) in which we've fixed the checkbox animation issue by using transition instead of animation (thanks @Catrope).

There's a loss of :focus feedback in the new widget for keyboard, AT users over OOUI's CheckboxMultiselectWidget!
Filed T249888: GrowthExperiments (/ MachineVisions) suggestions widget doesn't provide visual `:focus` feedback for keyboard/AT users