Page MenuHomePhabricator

Present preferences checkboxes as on/off toggles on mobile web
Closed, ResolvedPublic3 Estimated Story Points

Description

As part of our redesign of submenus in mobile preferences, we want to update the style of checkboxes to be presented as on/off toggles instead.

Design

Full image (PDF) detailing changes and specifications https://commons.wikimedia.org/wiki/File:Desing-specs-mobile-web-preferences.pdf.

For ON/OFF (or binary) options we will opt for a commonly used mobile component, the toggle switch.

Screen Shot 2022-06-24 at 11.49.50.png (334×1 px, 82 KB)

Acceptance criteria

  • All single checkboxes should be presented as on/off toggle switches in mobile web preferences
  • Toggling a preference on/off should have the same behaviour as toggling the checkbox

Event Timeline

Samwalton9-WMF set the point value for this task to 3.

This toggle style is already used in Special:MobileOptions when logged in, for comparison.

ERayfield changed the task status from Open to In Progress.Sep 19 2022, 11:16 AM
ERayfield claimed this task.

What we have learned so far:

https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/includes/preferences/DefaultPreferencesFactory.php is the code that contains the checkboxes we want to represent as toggles.

We can find an example of a toggle implementation here

https://gerrit.wikimedia.org/g/mediawiki/extensions/MobileFrontend/+/f90430d23fe697273743a38263cad4fafc8e5e0b/includes/specials/SpecialMobileOptions.php in the buildAMCtoggle() function

Here is the supporting JS file for the above php page. I think the trick here is to make sure the input becomes "infuseable" so that it can be changed later with JS to a toggle.

https://gerrit.wikimedia.org/g/mediawiki/extensions/MobileFrontend/+/f90430d23fe697273743a38263cad4fafc8e5e0b/src/mobile.special.mobileoptions.scripts.js

example http://localhost:8080/w/index.php?title=Special:MobileOptions&returnto=Special%3APreferences

http://localhost:8080/w/index.php?title=Special:MobileOptions&success

<div class="oo-ui-widget oo-ui-widget-enabled oo-ui-toggleWidget oo-ui-toggleWidget-on oo-ui-toggleSwitchWidget" aria-checked="true" tabindex="0" role="switch"><span class="oo-ui-toggleSwitchWidget-glow"></span><span class="oo-ui-toggleSwitchWidget-grip"></span></div>

Current
<input type="checkbox" tabindex="0" name="wpfancysig" value="1" id="ooui-php-26" class="oo-ui-inputWidget-input">

additional notes, pics located https://docs.google.com/document/d/1WtkONRJZ-vPX7w6MiGVnRB0rJBlouP36fPsQZSTF08Y/edit?usp=sharing

<input type="checkbox" tabindex="0" name="wpskin-responsive" value="1" checked="checked" id="ooui-php-43" class="oo-ui-inputWidget-input"> the type “checkbox” and oo-ui-imputWidget-input show a formatted checkbox

Code for Preferences
includes/preferences/DefaultPreferencesFactory.php
uselang=qqx results

I have done some research on how this is solved in Special:MobileOptions, and found that the checkbox is replaced with the toggle in JS. You can find the function here: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/refs/heads/master/src/mobile.special.mobileoptions.scripts.js#125.

You can see the hidden checkbox in this screenshot:

Screen Shot 2022-10-26 at 19.13.35.png (107×996 px, 16 KB)

What I suggest is doing the same thing in https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/resources/src/mediawiki.special.preferences.ooui/mobile.js.

Additionally, you might need to make each checkbox item infusable in https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/includes/preferences/DefaultPreferencesFactory.php.

From some back channel discussion between Ellen and I:

$defaultPreferences['fancysig'] = [
   'type' => 'toggle',
   'label-message' => 'tog-fancysig'.' HODOR',
   // show general help about signature at the bottom of the section
   'help-message' => 'prefs-help-signature',
   'section' => 'personal/signature'
];

The code snippet above is a preference descriptor element:
https://doc.wikimedia.org/mediawiki-core/master/php/DefaultPreferencesFactory_8php_source.html#l00735
that describes a checkbox/toggle. That toggle type gets related to the checkbox form element in the HTMLForm type mapper:
https://doc.wikimedia.org/mediawiki-core/master/php/HTMLForm_8php_source.html#l00165
The descriptor array is the thing that all the extensions add their preferences into

The form class in the default factory:
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/includes/preferences/DefaultPreferencesFactory.php#1820
Points to the Preferences form that we've been working on:
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/includes/specials/forms/PreferencesFormOOUI.php

Susana's advice to add infusable makes sense to me. Infuse transforms the php widget markup into a live javascript object that you can do fancy stuff with:
https://doc.wikimedia.org/oojs-ui/master/js/#!/api/OO.ui.Element-method-infuse
turning a checkbox into a toggle is one of those fancy things

the mobilefrontend code that susana pointed to is an implementation of the js ooui ToggleSwitchWidget https://doc.wikimedia.org/oojs-ui/master/js/#!/api/OO.ui.ToggleSwitchWidget

Basically, it gathers up those HTMLCheckFields (which come from type = toggle) and transforms them into toggle switches.

so in our example, we have this in the factory

$defaultPreferences['fancysig'] = [
   'type' => 'toggle',
   'label-message' => 'tog-fancysig'.' HODOR',
   // show general help about signature at the bottom of the section
   'help-message' => 'prefs-help-signature',
   'section' => 'personal/signature'
];

which is mapped to this type:
https://doc.wikimedia.org/mediawiki-core/master/php/classHTMLCheckField.html

I would look for a way to call the getInputOOUI() method on each HTMLCheckField instance in a loop in the mobile prefs form
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/includes/specials/forms/PreferencesFormOOUI.php#26
I'm not sure exactly where to instantiate the objects in that preferences descriptor array to run the method, but oing so returns an instance of OOUI\CheckboxInputWidget

In the mobile frontend code, the options menu builds CheckboxInputWidget instances, such as the amc toggle

			$amcToggle = new OOUI\CheckboxInputWidget( [
				'name' => 'enableAMC',
				'infusable' => true,
				'selected' => $userMode->isEnabled(),
				'id' => 'enable-amc-toggle',
				'value' => '1',
			] );

https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/refs/heads/master/includes/specials/SpecialMobileOptions.php#88

because that's set as infusable, each of those objects can be used in javascript, to replace checkboxes with toggles, like here:

/**
 * Helper method to infuse checkbox elements with OO magic
 * Additionally it applies all known hacks to make it mobile friendly
 *
 * @param {Object[]} toggleObjects an array of toggle objects to infuse
 * @param {jQuery.Object} $form form to submit when there is interaction with toggle
 */
function infuseToggles( toggleObjects, $form ) {
	toggleObjects.forEach( function ( toggleObject ) {
		var
			$toggleElement = toggleObject.$el,
			toggleSwitch,
			enableToggle,
			$checkbox;
		enableToggle = OO.ui.infuse( $toggleElement );
		$checkbox = enableToggle.$element;
		toggleSwitch = new OO.ui.ToggleSwitchWidget( {
			value: enableToggle.isSelected()
		} );
		// Strangely the ToggleSwitchWidget does not behave as an input so any change
		// to it is not reflected in the form. (see T182466)
		// Ideally we'd replaceWith here and not have to hide the original element.
		toggleSwitch.$element.insertAfter( $checkbox );
		// although the checkbox is hidden already, that is done via visibility
		// as a result, it still takes up space. We don't want it to any more now that the
		// new toggle switch has been added.
		$checkbox.hide();
		// listening on checkbox change is required to make the clicking on label working.
		// Otherwise clicking on label changes the checkbox "checked" state
		// but it's not reflected in the toggle switch
		$checkbox.on( 'change', function () {
			// disable checkbox as submit is delayed by 0.25s
			$checkbox.attr( 'disabled', true );
			toggleSwitch.setValue( enableToggle.isSelected() );
		} );
		toggleSwitch.on( 'change', function ( value ) {
			// execute callback
			toggleObject.onToggle( value );
			// ugly hack, we're delaying submit form by 0.25s
			// and we want to disable registering clicks
			// we want to disable the toggleSwitch
			// but we cannot use setDisabled(true) as it makes button gray
			toggleSwitch.setValue = function () {};
			$checkbox.find( 'input' )
				.prop( 'checked', value );
			notify( true );
			// On some Android devices animation gets stuck in the middle as browser
			// starts submitting the form.
			// Let's call submit on the form after toggle button transition is done
			// (0.25s, defined in OOUI)
			setTimeout( function () {
				$form.trigger( 'submit' );
			}, 250 );
		} );
	} );
}

https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/refs/heads/master/src/mobile.special.mobileoptions.scripts.js#125

We could implement something similar in the js that is called when mobile amc users access the preferences page and get our layout.
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/resources/src/mediawiki.special.preferences.ooui/mobile.js

Change 851647 had a related patch set uploaded (by EllenR; author: EllenR):

[mediawiki/core@master] WIP T317117 ckbox to toggle mobile

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

Still WIP, am currently trying to figure out how to apply the information from code that already has a toggle active on the special Advanced Mobile Contributions (AMC or Amc) option. Once that is diagramed, I'll sketch out the code where I want to put the toggle (sig section first as proof of concept), and sketch out where the rest of the code should go. As many know, it takes time to diagram out a specific section of code, but should make it easier to understand what is happening -

Ellen and I looked at this today and played with live HTMLCheckBox objects and examined the form output and determined that we already had access to the ooui version of the checkbox fields. Since that was the case, we only needed to work in js. I hastily adapted the readers web code that susana suggested to get a partially working example. There is still work to do there to finish out the event handling (currently checkboxes can only be enabled, not disabled) and to remove any extraneous code that we might not need.
Example change here:
https://gerrit.wikimedia.org/r/c/mediawiki/core/+/853314

As Jason says, there is still much work to do, the toggle displays, but has a few issues with switching from on to off, or really from one state to another. Also need some verification that our updates are working. Wow, who knew that javascript controlled all of this!

status update :
now have all checkboxes displaying as toggle in resources/src/mediawiki.special.preferences.ooui/mobile.js
all checkboxes turned to toggle will display on and off
still not quite ready for prime time as not saving state correctly

Change 855030 had a related patch set uploaded (by EllenR; author: EllenR):

[mediawiki/core@master] WIP mobile pref checkbox to toggle

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

Ellen and I looked at this together and determined that we weren't passing the disabled state from the checkbox. The toggles that weren't saving should have been grayed out. Here's an example of doing that end then returning out of that step of the loop for performance

		$( 'span.oo-ui-checkboxInputWidget' ).each( function () {
			var enableToggle = OO.ui.infuse( $( this ) );
			var toggleSwitch = new OO.ui.ToggleSwitchWidget( {
				value: enableToggle.isSelected(),
				disabled: enableToggle.disabled
			} );
			var $checkbox = enableToggle.$element;
			toggleSwitch.$element.insertAfter( $checkbox );
			$checkbox.hide();
			if ( toggleSwitch.disabled ) {
				return;
			}

It would be worth checking to see if there are any other checkbox properties that we should pass into the toggle upon creation.

Epilog

The major issues with getting this ticket done was not knowing where the code to change was located, and tuff documentation on OOUI. Started with looking at DefaultPreferencesFactory.php and thought the change should be in PHP code, if you have made it this far in the comments, you know that NOTHING that was changed was in the PHP code, it was all in the JavaScript. I am leaning towards thinking that if it is a frontend change which needs to happen, look to the JavaScript code first, CSS second, then PHP code. At least it seems that way in mobile preferences.

The fix was all done at the JavaScript level, by infusing the section, then hiding the checkbox and inserting code to enable the toggle switch.

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

Don't think I set that patch demo up correctly, sorry if it was premature 🤔

Change 851647 abandoned by EllenR:

[mediawiki/core@master] WIP T317117 ckbox to toggle mobile

Reason:

wrong track for change

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

With the advice to use plain JavaScript rather than jQuery, the code for the toggle switches needs to be adjusted.

Following the When (not) to use jquery - Mod Tools, I was able to get the

var elements = document.querySelectorAll( ‘selector’ );
Array.prototype.forEach.call( elements, function ( el ) {
});

to work as follows:

var elements = document.querySelectorAll( 'span.oo-ui-checkboxInputWidget' );
Array.prototype.forEach.call( elements, function ( el ) {
  var checkboxinputDisplayedAsToggleSwitch = OO.ui.infuse( el ); ,,,

However in trying to redo the .hide() call in $checkboxBeingDisplayedAsToggleSwitch.hide();, by following the afore mentioned doc which ( at this time ) states:
Instead of:

$infusedEl.hide()

Try:

infusedEl.classList.add( 'hidden' );

I suspect I missed something in the translation but get error message - so I must have something set up wrong. Will need to get with team to see where I missed a step

STILL NOT READY FOR PRIME TIME

I'm going to go ahead and review this based on our original expectations and then offer a followup patch to show how we might improve it.

Change 855030 merged by jenkins-bot:

[mediawiki/core@master] mobile pref checkbox to toggle and save

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

I'm tentatively moving this to the done column, though I understand @Samwalton9 may have feedback. We have follow-up work in T323231 that should be completed before making any other changes. In the pursuit of trying to make task tracking clearer on the engineering side, I may have made things less clear on the product side.

Additional note -
Preferences->Watchlist->Edit watchlist->(click button) View and remove titles on your watchlist

Looked at specs, not in design that I saw, but it does say all checkboxes - want to verify that these still need to be done (sorry I didn't notice earlier)

image.png (662×374 px, 36 KB)

This is the generated html for the first checkbox:

<div class="oo-ui-fieldLayout-body">
	<span class="oo-ui-fieldLayout-field">
		<span class="oo-ui-widget 
                                       oo-ui-widget-enabled 
                                       oo-ui-inputWidget 
                                       oo-ui-checkboxInputWidget">
			<input type="checkbox"
					tabindex="0"
					name="wpTitlesNs0[]"
					value="Main Page"
					id="ooui-php-1"
					class="oo-ui-inputWidget-input">
			<span class="oo-ui-checkboxInputWidget-checkIcon  
                                               oo-ui-widget 
                                               oo-ui-widget-enabled
					       oo-ui-iconElement-icon 
                                               oo-ui-icon-check 
                                               oo-ui-iconElement 
                                               oo-ui-labelElement-invisible
					       oo-ui-iconWidget oo-ui-image-invert">
			</span>
		</span>
	</span>
	<span class="oo-ui-fieldLayout-header">
		<label for="ooui-php-1" class="oo-ui-labelElement-label">
			<a href="/wiki/Main_Page" title="Main Page">Main Page</a>
			<span class="mw-changeslist-links">
				<span>
					<a href="/w/index.php?title=Talk:Main_Page&action=edit&redlink=1" class="new" 
					   title="Talk:Main Page (page does not exist)">talk</a>
				</span>
				<span>
					<a href="/w/index.php?title=Main_Page&action=history" title="Main Page">history</a>
				</span>
			</span>
		</label>
	</span>
</div>

in mobile.js we looked for 'span.oo-ui-checkboxInputWidget', in the above code there is a span containing oo-ui-checkboxInputWidget
Following is the generated code for checkboxes in Edit watchlist

'<span class="oo-ui-widget 
                       oo-ui-widget-enabled 
                       oo-ui-inputWidget 
                       oo-ui-checkboxInputWidget">

Looking at the generated code prior to changing over to toggles, there does not seem to be any differences in the setup, for that matter both of the span classes look identical:

Following is the generated code for checkboxes PRIOR TO EDIT for toggles and the second span tag below shows the exact same configuration

<div class="oo-ui-fieldLayout-body">
	<span class="oo-ui-fieldLayout-field">
		<span class="oo-ui-widget 
                                      oo-ui-widget-enabled 
                                      oo-ui-inputWidget 
                                      oo-ui-checkboxInputWidget" 
                           id="mw-input-wpfancysig" 
                           data-ooui="">
			<input type="checkbox" 
				   tabindex="0" 
				   name="wpfancysig" 
				   value="1"
				   checked="" 
				   id="ooui-php-28"
				   class="oo-ui-inputWidget-input">
			<span class="oo-ui-checkboxInputWidget-checkIcon 
                                               oo-ui-widget 
                                               oo-ui-widget-enabled 
			                       oo-ui-iconElement 
                                               oo-ui-iconElement-icon 
                                               oo-ui-icon-check 
                                               oo-ui-labelElement-invisible 
			                       oo-ui-iconWidget oo-ui-image-invert">				
			</span>
		</span>
</span>

So, I guess there are 2 questions here

  1. Do the checkboxes in Edit watchlist need to be converted to toggles?
  2. Is the issue that we need to iterate down another level, if so how is that done in JavaScript?

Additional note -
Preferences->Watchlist->Edit watchlist->(click button) View and remove titles on your watchlist

Looked at specs, not in design that I saw, but it does say all checkboxes - want to verify that these still need to be done (sorry I didn't notice earlier)
So, I guess there are 2 questions here

  1. Do the checkboxes in Edit watchlist need to be converted to toggles?
  2. Is the issue that we need to iterate down another level, if so how is that done in JavaScript?

No need to address this page too - we're only concerned about Special:Preferences for the scope of this project!

weird, it appears to be a child of Pref, but ok

weird, it appears to be a child of Pref, but ok

While you can navigate to it from preferences, it's a separate page (Special:EditWatchlist) that we haven't considered in the rest of the preferences redesign. I think it would really warrant a fresh design pass if we decided we wanted to start working on it too!

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

https://patchdemo.wmflabs.org/wikis/95069ab8ce/w/