Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Page MenuHomePhabricator

Create PreferencesGetIconHook for extensions to add icons to preferences
Closed, ResolvedPublicSpike

Description

Original Spike:
In T311717: Display Special:Preferences as a vertical menu instead of tabs on mobile for AMC users we were able to use the resource loader within the special page to load icons sets, but we also need to allow extensions to specify their own icons, which was called out in a code review:

Esanders: Extensions should define their own icons, which means you'll need a way for the preference hook to add to this data.

Research:
Based on our (the moderator tools team) reading of GetPreferencesHook:
https://doc.wikimedia.org/mediawiki-core/master/php/interfaceMediaWiki_1_1Preferences_1_1Hook_1_1GetPreferencesHook.html

The things in the preferences description array should directly inform the html form fields related to a given preference. The only thing that I see existing at a higher level in that data structure are the section keys themselves, which are needed to correctly place those fields.
It looked to us like we'd need to update the signature of GetPreferences to accommodate additional data, and it wasn't immediately clear to us that there was a good way to do so in a backwards compatible way with existing implementations.

The other place where we saw something altering the top level of the preferences form is another hook, PreferencesGetLegendHook:
https://doc.wikimedia.org/mediawiki-core/master/php/interfaceMediaWiki_1_1Hook_1_1PreferencesGetLegendHook.html

Outcome:
We will add a new preferences hook, following the existing pattern.

Event Timeline

Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptSep 9 2022, 2:57 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

The clear way to handle this via hooks is simply to add a new preferences hook, which I'm currently experimenting with.

I initially tried naively adding a parameter to the GetPreferences hook and immediately found that hooks in mediawiki have tight signature validation (which this approach fails), which is a totally reasonable way to manage them as APIs. So to make the change that way would require versioning up the hook, which we don't want to do. Looking at the currently implemented hooks as well as the hooks documentation, I can see that we are only supposed to have one method per hook, so adding a new method isn't an option either. I could try sticking icon data into the existing preferences array for GetPreferences, but that's a very unsanitary way of doing things, even if I did get it to work.

The nice thing about taking the hook approach is that it creates nice documentation for extension developers to get section icons registered. We could also handle it via module styles defined for special preferences, which would be straightforward to implement, but it would take a little more work to point extension developers in the right direction for implementing icons.

One question that comes to mind is how do we handle multiple extensions defining different icons for the same section?

Also, do we actually want extensions to provide their own image files? What if those don't align with the design we have?

jsn.sherman moved this task from In Progress to Ready on the Moderator-Tools-Team (Kanban) board.

In answer to:

Also, do we actually want extensions to provide their own image files? What if those don't align with the design we have?

From ed:

Realistically an extension should only be setting an icon for a new section it is creating. If there is a conflict you'll probably end up with whichever hook runs last "winning", which is fine.

Change 841591 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/core@master] Add icons to Special:Preferences mobile layout

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

Change 841592 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/skins/MinervaNeue@master] Add styling for Special:Preferences icons

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

Change 841594 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/extensions/BetaFeatures@master] Add PreferencesGetIconHook

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

Change 841597 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/extensions/Echo@master] Add PreferencesGetIconHook

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

Change 842551 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/extensions/LiquidThreads@master] Add PreferencesGetIconHook

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

Change 842553 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/extensions/UploadWizard@master] Add PreferencesGetIconHook

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

Change 842554 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/extensions/Gadgets@master] Add PreferencesGetIconHook

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

Change 842556 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/extensions/OpenStackManager@master] Add PreferencesGetIconHook

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

Test wiki created on Patch demo by SCardenas (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/9fb48305b0/w

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

https://patchdemo.wmflabs.org/wikis/9fb48305b0/w/

Test wiki created on Patch demo by SCardenas (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/449f7c9958/w

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

https://patchdemo.wmflabs.org/wikis/449f7c9958/w/

Change 841591 merged by jenkins-bot:

[mediawiki/core@master] Add icons to Special:Preferences mobile layout

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

Test wiki created on Patch demo by SCardenas (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/51ef7df668/w

Change 841594 merged by jenkins-bot:

[mediawiki/extensions/BetaFeatures@master] Add PreferencesGetIconHook

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

Change 841597 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Add PreferencesGetIconHook

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

Change 842554 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Add PreferencesGetIconHook

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

Change 842553 merged by jenkins-bot:

[mediawiki/extensions/UploadWizard@master] Add PreferencesGetIconHook

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

Change 849199 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):

[mediawiki/extensions/CentralNotice@master] Add PreferencesGetIconHook

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

Change 842551 merged by jenkins-bot:

[mediawiki/extensions/LiquidThreads@master] Add PreferencesGetIconHook

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

jsn.sherman renamed this task from SPIKE [8hrs] investigate methods for extensions to add icons to preferences to Create PreferencesGetIcon hook for extensions to add icons to preferences.Oct 26 2022, 4:16 PM
jsn.sherman renamed this task from Create PreferencesGetIcon hook for extensions to add icons to preferences to Create PreferencesGetIconHook for extensions to add icons to preferences.

Change 842556 merged by jenkins-bot:

[mediawiki/extensions/OpenStackManager@master] Add PreferencesGetIconHook

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

Change 849199 merged by Scardenasmolinar:

[mediawiki/extensions/CentralNotice@master] Add PreferencesGetIconHook

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

Change 841592 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Add styling for Special:Preferences icons

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

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

https://patchdemo.wmflabs.org/wikis/51ef7df668/w/