Page MenuHomePhabricator

Replace settings checkboxes to ToggleSwitches
Closed, ResolvedPublic3 Story Points

Description

We currently use checkboxes for turning a setting on/off

Proposal
Let's use ToggleSwitches from OOUI instead. Note: a fallback no-JS path still needs to be supported.

Why?
They convey the off state better than just a square. Toggleswitches have become a norm for on/off switches on mobile including iOS and Android.

From

To

Acceptance Criteria

  • Toggle switch appears instead of checkbox
  • Save button is removed for JS users
  • Setting is saved when the user toggles the toggle switch
  • Non-JS users will use the older version of this page (with checkboxes and save button)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

@Nirzar Related to this is open T140812#3300282. It'd be good to get feedback in order to get to a conclusion there…

Jdlrobson assigned this task to Nirzar.Oct 31 2017, 4:58 PM

@Nirzar We estimated this between 1-3 points. Toggle switches tends to automatically save and it's not clear from the mocks whether this will be the new behaviour for the form or whether we will continue to expect the user to click the save button.

If this is the new behaviour we need to think about the behaviour on non-JS browsers... what happens there?

Nirzar closed this task as Declined.Nov 23 2017, 8:50 PM

declining for now.. toggle switches require async save functionality AND a non-js fallback for async.

we will pick this up as a follow up to the whole setting + beta work.

for now we will keep the checkboxes.

FYI async saving is quite trivial to add (in fact I added it as part of the POC patch).

I agree though non-js fallback for this control is a little more confusing... unless you switch a checkbox to a toggle switch which might be jarring to a user.

FYI async saving is quite trivial to add (in fact I added it as part of the POC patch).

that's good to know!

I guess async + non-js fallback to checkboxes and save button would be a little too much work right now. it won't be confusing given that non-js usually has different experience.

how would categorize following work, small, m, l, xl?

for JS

  • show toggle switches for controls
  • remove save button
  • save settings async

for non-js

  • show checkboxes for controls
  • keep save button
Nirzar reopened this task as Open.Dec 1 2017, 1:40 AM

@Jdlrobson based on staging, if we are able to actually do the async save easily then I would like to reopen this card and use toggle switches. let's keep this in the backlog and see how much effort that is. we will need toast message saying "Your preference is saved" for async saved, so we will need new card for async save as well. per @ovasileva

OOJSUI integration is covered in T169369 which includes extra performance AC. This task probably shouldn't be worked on until that's done.

Change 394634 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@specialpages] Upgrade checkboxes to ToggleSwitches on options form

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

Jdlrobson added a comment.EditedDec 1 2017, 7:00 PM

@Nirzar the ToggleSwitches could be done like this: http://reading-web-staging.wmflabs.org/wiki/Special:MobileOptions
Checkboxes get upgraded to toggle switches via JS. Note there is an unavoidable flash of unstyled content when doing this (as a result of needing to support non-JS users)

Thoughts on flash of unstyled content when we switch checkboxes to toggle switches?

Nirzar added a comment.EditedDec 5 2017, 4:35 PM

Oh sorry, I missed this comment. I would want to see the FOUC we are talking about. I should have checked it before when toggleswitches were deployed.

another option is not to show checkboxes and only show toggleswitches after loading. till then keeping the visibility:invisible

Yeh we can do that, but trade off is that if there's a bug in OOjs UI the form becomes unusable, but I guess with async saving we have that problem anywhere. Looks like we have all we need to do this. Just needs to be discussed, estimated and brought into the sprint.

ovasileva updated the task description. (Show Details)Dec 6 2017, 5:23 PM
ovasileva updated the task description. (Show Details)
ovasileva set the point value for this task to 3.Dec 6 2017, 5:31 PM
phuedx updated the task description. (Show Details)Dec 6 2017, 5:32 PM

It looks like the beta mode needs to be saved by submitting the form.
@Nirzar this creates problems as the ToggleSwitch is not a checkbox so it doesn't update any form value that can be submitted (thus beta optin currently not working on staging :()

I think we need to sync about how saving should work and the work involved to make beta behave asynchronously and whether the effort to fix this is worth it.

See also https://phabricator.wikimedia.org/T169369#3823601

Jdlrobson removed Nirzar as the assignee of this task.Dec 8 2017, 8:57 PM

After chatting with @Nirzar today we agreed that we will go ahead with making the beta toggle save on the server.

Please review Upgrade checkboxes to ToggleSwitches on options form, verifying that the form still functions. The behavior is satisfactory. Any findings(bugs/UX problems) during code review can be fed back to Nirzar once the code is merged.

Latest and greatest code is on https://reading-web-staging.wmflabs.org/wiki/Special:MobileOptions

https://gerrit.wikimedia.org/r/#/c/396082/6 is merged. I have some strange issues while testing https://gerrit.wikimedia.org/r/394634 - save behavior is wonky, sometimes it saves the value, sometimes it doesn't. @Jdlrobson cannot reproduce it locally. I want to make sure that this behavior is specific only to my machine

pmiazga claimed this task.Dec 13 2017, 1:41 PM

I'm still investigating the issue

Volker_E updated the task description. (Show Details)Dec 15 2017, 2:37 AM
pmiazga removed pmiazga as the assignee of this task.Dec 15 2017, 3:28 PM
pmiazga removed a project: Patch-For-Review.
pmiazga moved this task from Doing to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.

Change 394634 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Upgrade checkboxes to ToggleSwitches on options form

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

@pmiazga to updating the staging server and ping a signerer offerer.

just spoke with @pmiazga about 3 points

summary -

  • Beta settings toggle should not get stuck on refresh page, figuring out other way to enable beta mode
  • Look into delay before the toast message, it's localstorage and there shouldn't be any delay for toast to show when a setting is saved
  • tapping on <label> doesn't trigger the input element on some androids

@Nirzar

  • Beta settings toggle should not get stuck on refresh page, figuring out other way to enable beta mode

We might do it properly with async save, not the page reload. I'm not touching it right now.

  • Look into delay before the toast message, it's localstorage and there shouldn't be any delay for toast to show when a setting is saved

There is 1s delay between action and showing the toast message. I'll remove that delay

  • tapping on <label> doesn't trigger the input element on some androids

I'll look at it, so far I see that "Expand all sections" label is missing for attribute.

Change 398530 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@specialpages] Show toast messages without 1s delay

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

Change 398530 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Show toast messages without 1s delay

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

  • Look into delay before the toast message, it's localstorage and there shouldn't be any delay for toast to show when a setting is saved

This is done.

Change 398889 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@specialpages] Toggle the Beta toggle when user clicks on label

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

pmiazga assigned this task to bmansurov.Dec 18 2017, 6:11 PM
pmiazga moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.
bmansurov reassigned this task from bmansurov to pmiazga.Dec 18 2017, 8:59 PM
bmansurov added a subscriber: bmansurov.

I've left a comment on the patch.

Change 398889 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Toggle the Beta toggle when user clicks on label

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

pmiazga reassigned this task from pmiazga to Nirzar.Dec 20 2017, 6:03 PM

Change 399463 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@specialpages] Delay form submit by 0.25s

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

I decided to quickly patch

Beta settings toggle should not get stuck on refresh page, figuring out other way to enable beta mode

by adding 250ms delay between toggle change and form submit.

We don't know when (and if) we're going to change the Special:MobileOptions save mode to async and we shouldn't leave it as it is right now.

The delay is making the experience much better. I can toggle the switch now. looks good.

Change 399463 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Delay form submit by 0.25s

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

Nirzar closed this task as Resolved.Jan 2 2018, 9:54 PM

Macro votecat: looks good