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

The engineering team looked at this demo today and we talked through quite a few options on how the save button could or should work as well as how autosave could or should work given that this is a multilevel form with navigation. For now, we think the save button always being shown at the bottom, even at the top-level list is the way to go for the mvp, and that looking at the save behavior and ux is worth its own spike. This seems to line up with what's been said in thread here so far, but I just also wanted to note the engineering outlook on it. I'm happy to create a draft spike for saving and/or autosaving that includes the options we discussed so far.

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

Change 829064 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):

[mediawiki/core@master] [WIP] Mobile Preferences - resource loader demo

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

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

https://patchdemo.wmflabs.org/wikis/4cf6266f9f/w/

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

@AAlhazwani-WMF If you have time it would be great to get some quick thoughts on whether this looks and behaves like you anticipated! We reduced the scope of the ticket a little - submenus currently open inside the preferences area, rather than being full-screen modals, so this ticket is limited to the main menu layout and design.

Instead of creating a new paintbrush icon for "Appearance" there are some existing icons that might work:

In T311717#8208453, @Esanders wrote:

Instead of creating a new paintbrush icon for "Appearance" there are some existing icons that might work:

We have a final design for a new palette icon at T311514 :)

Change 829064 abandoned by Jsn.sherman:

[mediawiki/core@master] [WIP] Mobile Preferences - resource loader demo

Reason:

demonstration purposes only

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

This is officially ready for review!

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

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

https://patchdemo.wmflabs.org/wikis/055fb222d9/w/

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

https://patchdemo.wmflabs.org/wikis/c34aff28ab/w/

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

Remaining changes +2ed

Change 822697 merged by jenkins-bot:

[mediawiki/core@master] Mobile Preferences - display Special:Preferences as a vertical menu

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

Change 822698 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Mobile Preferences: Add styles for Special:Preferences mobile

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

Change 823271 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Change configuration variable for mobile view in Special:Preferences

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

Change 822697 merged by @jsn.sherman:

[mediawiki/core@master] Mobile Preferences - display Special:Preferences as a vertical menu

[…]

It also creates a variable (wgSpecialPreferencesUseMobileLayout) that changes when a user navigates to Special:Preferences on mobile and has AMC enabled.

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

@jsn.sherman I don't think this is what configuration variables are for and is in my view a problematic contract to try to support in MediaWiki core. Even if we were to accept the risk of trying to deploy this, I'm actually not sure it's even technically possible as we load our configuration much before concepts like MobileFrontend mode and user preferences are available. (Is there a draft wmf-config patch?)

I suggest asking @Esanders for help as you're already interacted here, but I'm happy to help further as well.

@Krinkle thank you for the additional information. It sounds like we need to kick this back and do some more learning.

@Esanders our thinking here is:
We have a default config in which the current preferences form is displayed. If the user has AMC enabled and is in the mobile view, we flip the value to show the alternative form.

Are we misunderstanding how to use config?

In T311717#8240631, @jsn.sherman wrote:

@Krinkle thank you for the additional information. It sounds like we need to kick this back and do some more learning.

In T311717#8240558, @gerritbot wrote:

Change 823271 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Change configuration variable for mobile view in Special:Preferences

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

I'm happy to let Ed recommend something different, but from a quick look here, my suggestion would be to replace the newly added config var with a hook. Your code in core would run the hook at the known appropriate time (e.g. the line where we currently read the config var). Then, MobileFrontend naturally reacts to that hook at that time and e.g. uses onPreferencesFormLayout( $context, $layout ) to change "default" to "mobile". Perhap using public constants for improved static validation, usage tracking (possibly deprecation in the future), and easy way to document that no other values are supported.

The above is just one way to do it, more important are these observations:

• The patch currently relies on deprecated $wg variables. In new code, we use the MainConfig service to access configuration instead.
• When we did use globals for new code, modifying configuration at runtime was not supported. This is what hooks are for.
• Hooks avoid the unsupported mutation of config variables, which may stop working any time without warning.
• Hooks also avoid creating a developer experience where debugging of site configuration is incomplete or not always the same for a given wiki/environment, and not discoverable through configuration files.
• The patch currently performs tight coupling, where MobileFrontend is forced to try to predict a time that is "late" enough to be safe, yet "early" enough to still be before core reads the variable. I can see how you arrived at onRequestContextCreateSkin, as that's exactly the spot that happens to satisfy that condition. That hook is also a classic in my spider sense of likely being indicative of technical debt, hence I'm trying to nudge away from that in new code. If we do find there's no good way to avoid it, I'll take that as a sign that we need to provide something new to facciliate your use case.
• The current patch might stop working when we improve core PreferencesForm to follow modern DI practices (learn more) , where config variable are often read once during service wiring or construction, instead of later at runtime.

Ed's feedback about how extensions need to interact with this also has me looking at how mediawiki implements hooks. It's clearly our (moderator tools) team's next step in understanding mediawiki.

I opened reverts for these two changes.

Reverts are merged.

Nit design review. I tweaked a couple of CSS properties with the web inspector to get closer to the desired result. For simplicity I used px values, but feel free to convert those to em/rems as necessary.

.ns-special .heading-holder {
  padding-top: 12px;
}

.oo-ui-iconWidget {
  width: 20px;
  height: 20px;
}

.mw-prefs-icon {
  margin-right: 8px!important;
}

.content h5 {
  padding-bottom: 4px; 
}

.content p {
  margin-bottom: 12px;
}

.mw-prefs-description {
  padding: 0 0 0 28px; // 20px icon + 8px margin right
}

.mw-mobile-preferences-option {
  padding-top: 12px;
}

.page-heading {
  margin-bottom: 0; // because of .mw-mobile-preferences-option padding top
}

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

[mediawiki/extensions/MobileFrontend@master] Add hook to check if Special:Preferences should have mobile layout

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

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

[mediawiki/skins/MinervaNeue@master] Improve Special:Preferences mobile styles

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

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

[mediawiki/core@master] Redesign Special:Preferences for mobile

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

Hi @Krinkle! I have reworked this ticket based and your feedback and is ready for any comments or feedback you have.

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

Sorry to leave this comment on the doorstep a couple days down the road of implementation. Why did the team discount and/or not respond to my comment at the request for feedback page about making sure this can scale up/past Minerva/MobileFrontend? Minerva may not be the only skin that wants mobile-styled preferences (Timeless sure does, though what it has today "works"). Whatever the answer for the first question, is this path something other skins could use, or does it actually require MobileFrontend?

(I have the general opinion that MF should be exclusive to "hacks" in the modern day, and this doesn't look like one to me today especially since you plan to release this for all users at some point, meaning it should be a change solely to Minerva or to some other upstream Skin Component, to make it available for other skins.)

@Izno I can't speak for anyone on the design side (we're not following the design talk pages), but on the engineering side, it's not my intention to leave this tightly coupled to minerva. As a relative newb to mediawiki software, I can tell you that it's challenging to do something like a single responsive design for something like preferences, because most of the markup comes out of core and the default experience is not responsive. We are attempting to start small and iterate in order to get basic features such as password change out to mobile users sooner rather than later, and minerva is an obvious place to start since it's what users see by default in mobilel on production mw projects.

@jsn.sherman I think what @Izno is suggesting is what I mentioned on Slack and Gerrit about skin-specific config. The benefit of using skin-specific config is you can target the Minerva skin, but you enable other skins to use it in future too as needed (e.g. Timeless). The problem with MobileFrontend is it only applies to the mobile domain. I think you can meet your goals of starting small and iterating by applying this change via configuratio to preferences page unconditionally to the Minerva skin. The Minerva skin has always been a good experimental ground for UI changes that have latter been adopted in other skins. Happy to chat more if something about that approach is of concern!

In T311717#8251281, @jsn.sherman wrote:

it's not my intention to leave this tightly coupled to minerva

As Jon implies, my primary concern actually isn't coupling to Minerva (that's secondary), it's coupling to MobileFrontend. All skins should want a mobile-friendly resolution for settings and the experience out of core should support that (at least at a minimal level), rather than MF, or indeed in Minerva (secondarily).

Yes, I have no issue with this being an iterative moving. I did want to ensure that my earlier feedback had been heard, as going by this task description laying out specific objectives, it had not been (and maybe I just hadn't seized on the right task to comment on ;).

@Izno the only reason we're using mobile frontend here is to start out with mobile users who have AMC enabled, which is a mobileFrontend feature.

@Jdlrobson I may be misunderstanding @Krinkle's feedback, but I read this as a strong suggestion to look elsewhere besides config for this behavior in the preferences form:

I don’t think this is what configuration variables are for and is in my view a problematic contract to try to support in MediaWiki core. Even if we were to accept the risk of trying to deploy this, I’m actually not sure it’s even technically possible as we load our configuration much before concepts like MobileFrontend mode and user preferences are available. (Is there a draft wmf-config patch?)

When I looked for other examples of skin config, I found them in extensions, but not core. Doing the skin config implementation would eliminate the runtime config change, but I took this feedback as "don't, even if you can."

@Izno the only reason we're using mobile frontend here is to start out with mobile users who have AMC enabled, which is a mobileFrontend feature.

@jsn.sherman the preferences page is not available from the standard mobile interface so does this really need to be limited to the AMC mode in Minerva?

When I looked for other examples of skin config, I found them in extensions, but not core. Doing the skin config implementation would eliminate the runtime config change, but I took this feedback as "don't, even if you can."

wgSkipSkins is an example of configuration in core that considers the skin name. SpecialPreferences class has knowledge of the skin so would be able to change its output based on what skin is present.

@jsn.sherman the preferences page is not available from the standard mobile interface so does this really need to be limited to the AMC mode in Minerva?

That is a good question. I suppose we could just enable/disable the menu item in mobilefrontend and otherwise leave it decoupled from that extension. This has definitely been evolving as we get to grips with the complexity of the deployed environment as long as there is a way for skins to be opted in to the mobile layout (which of course the skin config would do).

wgSkipSkins is an example of configuration in core that considers the skin name. SpecialPreferences class has knowledge of the skin so would be able to change its output based on what skin is present.

Thank you!

I would definitely also like to hear back from @Krinkle as well, to see if we were missing the scope of the provided feedback. As a team, we are still trying to grow our understanding of and competency in dealing with mediawiki, so if there is something we need to learn about the limitations and appropriate use of config, I'd rather learn it sooner rather than later.

the preferences page is not available from the standard mobile interface so does this really need to be limited to the AMC mode in Minerva?

Just to add some quick context here - we're only limiting to AMC initially for user testing purposes, and we're aiming to remove that restriction once we're happy with the user experience. This patch goes alongside T311720, which does actually make the preferences page available from the standard mobile interface (also for AMC users initially).

If that constrains the way we implement this feature in ways that aren't productive then I'm happy to discuss not taking that path.

Hello @Izno and @Jdlrobson. I have been making some tests to see if the changes in the patches are mobile skin agnostic. These are some of my findings:

1. I may have inadvertently made this skin agnostic while trying to cover the edge case where you can force a skin and mobile format via URL parameters. I added some CSS in core's preferences styles file so that if a user navigates to Special:Preferences?usekin=timeless&useformat=mobile, the proper mobile layout is shown. You can check this behavior in the latest patch demo I created. I have tested this with Timeless, Vector, and Monobook and all seem to be working correctly.

2. I have just tested adding Timeless as my default mobile skin in my local environment. When I switched from desktop to mobile view, the Timeless skin correctly displayed the mobile layout.

3. I think the UI in non-Minerva skins is not as polished but could be improved if a CSS file is added to the skin to fix UI flaws like we did in Minerva.

Based on my findings, do you think these changes are skin agnostic? Let me know if you have any further questions or comments.

In T311717#8252165, @Scardenasmolinar wrote:

1. I may have inadvertently made this skin agnostic while trying to cover the edge case where you can force a skin and mobile format via URL parameters. I added some CSS in core's preferences styles file so that if a user navigates to Special:Preferences?usekin=timeless&useformat=mobile, the proper mobile layout is shown. You can check this behavior in the latest patch demo I created. I have tested this with Timeless, Vector, and Monobook and all seem to be working correctly.
2. I have just tested adding Timeless as my default mobile skin in my local environment. When I switched from desktop to mobile view, the Timeless skin correctly displayed the mobile layout.

[snip]

Based on my findings, do you think these changes are skin agnostic? Let me know if you have any further questions or comments.

Skin agnostic, kind of looks like it. Yay!

But, both items 1 and 2 indicate the remaining dependency on MobileFrontend. I'm suggesting that the final state would be that I would navigate to https://www.mediawiki.org/wiki/Special:Preferences?useskin=minerva (i.e. neither the mobile domain nor the mobile-format forcing) below the mobile cutoff which would cause the proposed design would display, and above that cutoff we would see the 'normal' desktop preferences page. (Both Minerva and Timeless have mobile cutoffs somewhere in their LESS.) I do not know how feasible that is but expect it's in the realm of "can be done, but more work than this task implies". (The implication is that if that can't be done now, it's one of those things that inhibits objectives like having a skin scale without regard for MobileFrontend i.e. technical debt.)

(There's a niggling thought that maybe it would just be categorically better to have this be how preferences are displayed, but that is decidedly a larger social question than what this task and task chain should be. ;)

Change 834066 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):

[mediawiki/skins/MinervaNeue@master] [WIP] Use PreferencesGetLayout hook to for mobile prefs

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

In T311717#8253342, @gerritbot wrote:

Change 834066 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):

[mediawiki/skins/MinervaNeue@master] [WIP] Use PreferencesGetLayout hook to for mobile prefs

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

This was just me demonstrating implementing the hook in the minerva skin to toggle the layout without mobilefrontend loaded. Works as expected.

In T311717#8252549, @Izno wrote:

I'm suggesting that the final state would be that I would navigate to https://www.mediawiki.org/wiki/Special:Preferences?useskin=minerva (i.e. neither the mobile domain nor the mobile-format forcing) below the mobile cutoff which would cause the proposed design would display, and above that cutoff we would see the 'normal' desktop preferences page. (Both Minerva and Timeless have mobile cutoffs somewhere in their LESS.) I do not know how feasible that is but expect it's in the realm of "can be done, but more work than this task implies". (The implication is that if that can't be done now, it's one of those things that inhibits objectives like having a skin scale without regard for MobileFrontend i.e. technical debt.)

If this isn't currently (or by the time this is merged) the case then I think it's a sensible suggestion and I'd be happy to file a follow-up task.

@Jdlrobson, what do you think of using implementing this hook in minerva, like https://gerrit.wikimedia.org/r/834066 along with fleshing out https://gerrit.wikimedia.org/r/c/833471 to the point where it no longer depends on mobilefrontend at all? Then we are only using mobilefrontend to control whether or not preferences is surfaced in the menu. I know the hook approach is different than the skin config approach you suggested, but are there problems with doing it this way?

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

[mediawiki/extensions/BetaFeatures@master] Add a description for new Special:Preferences layout

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

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

[mediawiki/extensions/CentralNotice@master] Add description for Special:Preferences layout

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

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

[mediawiki/extensions/Echo@master] Add a description for Special:Preferences layout

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

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

[mediawiki/extensions/Gadgets@master] Add a description for Special:Preferences

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

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

[mediawiki/extensions/OpenStackManager@master] Add description for Special:Preferences

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

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

[mediawiki/extensions/LiquidThreads@master] Add a description for Special:Preferences

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

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

[mediawiki/extensions/UploadWizard@master] Add description for Special:Preferences

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

Change 835672 merged by jenkins-bot:

[mediawiki/extensions/OpenStackManager@master] Add description for Special:Preferences

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

@AAlhazwani-WMF We've identified a need for a fallback icon if an extension adds a preferences section that doesn't have an icon defined. Do you have any suggestions about what we should use?

Change 835666 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Add a description for Special:Preferences

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

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

https://patchdemo.wmflabs.org/wikis/6c3f67261b/w/

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

https://patchdemo.wmflabs.org/wikis/c15f1060a8/w/

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

https://patchdemo.wmflabs.org/wikis/16c3b45110/w/

In T311717#8268072, @jsn.sherman wrote:

@AAlhazwani-WMF We've identified a need for a fallback icon if an extension adds a preferences section that doesn't have an icon defined. Do you have any suggestions about what we should use?

Vector 2022 uses this cog icon (I'm not sure what OOUI icon it is) for Preferences, so it could make sense as a generic fallback?

Change 835676 merged by jenkins-bot:

[mediawiki/extensions/LiquidThreads@master] Add a description for Special:Preferences

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

Change 835665 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Add a description for Special:Preferences layout

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

Change 835663 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Add description for Special:Preferences layout

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

Change 835677 merged by jenkins-bot:

[mediawiki/extensions/UploadWizard@master] Add description for Special:Preferences

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

Change 833471 merged by jenkins-bot:

[mediawiki/core@master] Redesign Special:Preferences for mobile

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

Change 833470 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Improve Special:Preferences mobile styles

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

@Samwalton9 looking through this with Susana and talking with Jon, we think calling the mobile layout directly in minverva, rather than mobileFrontend would be a good option. This is where we need to land once we're ready to roll out to everyone, so it would be cleaner to just start that way to start with if we can. Only AMC users will be presented with the preferences in the settings menu since that is controlled separately. This would change what non-AMC users see if they access Special:Preferences by search and would be technically out of scope with the ticket, so I'm checking to see if we can adjust scope on this. We already have both implementations on hand, so there is no real difference in the difficulty of the initial deployment.

Change 835655 merged by jenkins-bot:

[mediawiki/extensions/BetaFeatures@master] Add a description for new Special:Preferences layout

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

In T311717#8292470, @jsn.sherman wrote:

@Samwalton9 looking through this with Susana and talking with Jon, we think calling the mobile layout directly in minverva, rather than mobileFrontend would be a good option. This is where we need to land once we're ready to roll out to everyone, so it would be cleaner to just start that way to start with if we can. Only AMC users will be presented with the preferences in the settings menu since that is controlled separately. This would change what non-AMC users see if they access Special:Preferences by search and would be technically out of scope with the ticket, so I'm checking to see if we can adjust scope on this. We already have both implementations on hand, so there is no real difference in the difficulty of the initial deployment.

That's fine by me!

This isn't visible to users yet but the bulk of the work is merged. Two final tickets before making this available: T317419 and T320586

Change 833469 abandoned by Scardenasmolinar:

[mediawiki/extensions/MobileFrontend@master] Add hook to check if Special:Preferences should have mobile layout

Reason:

We went in another direction

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