Page MenuHomePhabricator

Update CentralNotice admin UI to fix or replace broken JQ multiselect widget
Closed, ResolvedPublic

Description

Removal of JQMigrate (see T280944) breaks the creaky old multiselect widget used to select languages and projects (see T291410)

One possible drop-in replacement multiselect, though it'll look different: https://ehynds.github.io/jquery-ui-multiselect-widget/#selectedlist

That one would have the advantage of working with the same HTML that our backend generates, while OOUI would require rewriting the selector construction.

Event Timeline

@Pcoombe that looks like exactly the cause! Locally I can see the breakage with that setting false, but the multiselect works properly with wgIncludejQueryMigrate = true;

There should be a helpful migration warning in the browser console when testing it locally (or on Meta-Wiki for that matter) when $wgIncludejQueryMigrate = true; This feature both restored the legacy methods as well as installed a deprecation message to be logged with what method exactly changed in jQuery 3, which would then allow you to make the needed change to the widget. It's usually a one-line change where a method name changed.

See also https://www.mediawiki.org/wiki/RL/MGU#jQuery_Migrate_3.

@Krinkle thanks much for the tip, super helpful! Also many apologies that this work was not prioritized...

Change 722441 had a related patch set uploaded (by Ejegg; author: Ejegg):

[mediawiki/extensions/CentralNotice@master] Replace obsolete jquery multiselect plugin with working one

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

Hi Design System folks! Just added your project tag to ask for suggestions on supported widgets (or combinations/adaptations of supported widgets) to use in place of the one we seem to have to remove. It's a multiselect widget with two scrollable lists, one of selected items and another of non-selected items, used to set projects and languages for CentralNotice campaign targeting. You can click on any campaign in the CN interface to see how the current widget works.

Many thanks in advance for any ideas or suggestions of where to go with this!! :)

Hi! Thanks @Ejegg for that patch!! Here is a summary of options we've discussed (from discussions on IRC and Gerrit):

  1. Replace old widget with a new one from elsewhere, similarly copying the code into the CN codebase. This is the option explored in the attached patch. The new widget provides less functionality than the existing one. A security review would be needed. (Also, I guess I also have reservations about copying a widget in like this.)
  2. Dive into the code of the existing widget. Maybe there are only a few bits that we could update easily to make it work without the jQuery migration layer? (That layer is what was just de-activated in the config that was then reverted; see T291410.)
  3. Use or adapt one or more OOUI widgets. For example, we could have a scrollable pannel with a multi-select checkbox widget, and add Select All, Select None and Filter controls, plus auto-generated text listing the selected options.

It seems VueJS widgets may not be ready for a use case like this. (Thanks @Jdforrester-WMF for this feedback!) Also, as per @Ladsgroup, it would be ideal to un-revert the config change on Meta within a few weeks (though I don't know how serious the consequences of a possible extension might be).

As next steps, what about maybe trying (2) and if that seems complicated, make some quick mockups of (3) to check with CN Admins?

Thanks so much, all!!

Also worth considering what would break if we specifically included JQ Migrate just on the CN campaign admin page

Change 722695 had a related patch set uploaded (by AndyRussG; author: AndyRussG):

[mediawiki/extensions/CentralNotice@master] Close all tags in multiselect for jQuery update

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

Change 722441 abandoned by Ejegg:

[mediawiki/extensions/CentralNotice@master] Replace obsolete jquery multiselect plugin with working one

Reason:

Looks like Andrew has a fix for the existing library!

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

In case we do ever need the actually-maintained successor to the existing library, it's here: https://github.com/yanickrochon/jquery.uix.multiselect

Change 722695 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Close all tags in multiselect for jQuery update

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

Just to note, the solution that was merged is approach (2), a small tweak to the existing widget.

Also worth considering what would break if we specifically included JQ Migrate just on the CN campaign admin page

Also a great idea, yeah!

Also, thanks in any case Design Systems folks... you may hear from us again... ;p

AndyRussG renamed this task from Update CentralNotice admin UI not to use broken JQ multiselect widget to Update CentralNotice admin UI to fix or replace broken JQ multiselect widget.Sep 24 2021, 3:00 PM
AndyRussG claimed this task.

The JQuery migration setting has now been turned off on Meta, and things are working fine. So I think we can close this. Thanks so much, everyone!!!