Page MenuHomePhabricator

Change font size controls to use small, regular, large and extra large buckets
Closed, ResolvedPublic5 Story Points

Description

Problem

The existing font changer on mobilefrontend uses +/- controls and a the feedback is given with a % value to the user. This is a complicated control to use and the user expectation of 100%/110% is difficult to grasp.

Solution

Create easy buckets of font sizes

  1. Small - 90%
  2. Regular - 100% (default)
  3. Large - 120%
  4. Extra Large - 140%

The buckets can be selected with a dropdown widget.

Developer notes

We'll introduce the OOUI library (PHP+CSS) on this page. JS should not be needed since this is a simple form.
Before implementing read through the task T180419. It may make sense to do that as part of the change or separately. Developer should make the call.

Acceptance criteria

  • OOUI CSS should not be loaded on any page other than Special:MobileOptions; OOUI JS is not loaded on any page
  • Document the change of size in CSS and JS on this page somewhere under mw.org Reading/Web so that we have data about what the impact of including OOUI is on a given page.
    • Send an email to the team list with the page

Testing

The changes can be verified on reading web staging:
http://reading-web-staging.wmflabs.org/wiki/Special:MobileOptions

They cannot be tested on the beta cluster or production.

Sign off criteria

  • Review T180419 to see which bits of it still apply
  • Make a decision on whether to merge the code from reading web staging to the beta cluster and thus production. If not, make sure to update follow up tasks with this statement.

When doing this consider the following

  • This will slow down the loading of the Special:MobileOptions page - do we want to investigate to what scale in any way?
  • This will impact all special pages - do any look strange?
  • Does it work without JS?
  • Is the async saving acceptable?

Sign off process:

  • Squash and merge the specialpages branch of MobileFrontend into master
  • Squash and merge the specialpages branch of MinervaNeue into master (with dependency on the MobileFrontend change)

Related Objects

Event Timeline

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

@Nirzar This appears to be in upcoming, but doesn't the design depend on T169379 which is not in upcoming? If not, it might be helpful and less confusing to have a mock showing the new font selector in the existing layout (given that it only shows in beta mode).

but doesn't the design depend on T169379 which is not in upcoming?

it doesn't

Updated the task description with mock that reflects the change.

also for reference the EPIC has the order of cards now https://phabricator.wikimedia.org/T67079

and for future reference, an open question is if we want to build out the page template or just change the page title styling? based on that we can decide whether to do that before we touch individual controls

Nirzar updated the task description. (Show Details)Oct 30 2017, 8:45 PM
Jdlrobson added a comment.EditedOct 30 2017, 9:23 PM

Thank you! Being able to see the order is very helpful.

Note Minerva/mediawiki ui has no concept of a drop-down widget and there's not one in mediawiki ui so I guess as part of this we will need to introduce OOjs UI on this page for all controls. On plus side this will make it possible to use the toggle widget thingy but on the downside it will mean we will bloat the CSS and JS for this specific page as mediawiki UI is a hard dependency of the minerva skin.

ovasileva set the point value for this task to 3.Oct 31 2017, 4:44 PM
Jdlrobson updated the task description. (Show Details)Oct 31 2017, 4:45 PM
Jdlrobson updated the task description. (Show Details)

Can we do a 4-button toggle (in a row, only one button can be selected). It would look like:

[Small][Regular][Large][ExtraLarge]

maybe the text on the button can reflect the size of the text. Then we wouldn't need an OOUI library. Once a user clicks one button it unselects the previous option and selects the clicked one.

Nirzar added a comment.EditedOct 31 2017, 4:51 PM

Can we do a 4-button toggle (in a row, only one button can be selected). It would look like:

not scalable if we need to add more options in future. horizontal components will have issues with long strings in few other languages.

as Jon said, it's good to use oojs ui so we can use the toggle switches and also we don't have to keep things in sync all the time.

I would like to know what we mean by bloat though? any rough estimates?

We've agreed to worry about bloat later. Any CSS bloat will be constrained to the Special:MobileOptions page and we can feed that back to the oojs ui team if necessary and we can reduce it by migrating mediawiki ui components here to ooui ones.

Jhernandez updated the task description. (Show Details)Nov 8 2017, 5:28 PM
Jhernandez updated the task description. (Show Details)
phuedx changed the point value for this task from 3 to 5.Nov 8 2017, 5:34 PM
phuedx added a subscriber: phuedx.EditedNov 8 2017, 5:38 PM

We bumped the estimate for this task as we'd also like to report on the increase in payload size caused by the introduction of the OOUI CSS blob internally and externally.

When we're reporting on this, we should bare in mind that we're going to be using OOUI for other controls on this page too. Any concern with an increase in size should be considered alongside the design concern to standardize controls across all experiences and that we're going to be using other parts of OOUI /cc @Nirzar

For measuring the CSS & JS sizes you can use mw.loader.inspect, or in an incognito window open the network tab and see the total sizes at the bottom after loading the mentioned page (something like https://m.mediawiki.org/wiki/Special:MobileOptions?mobileaction=beta)

I had a go at teasing out the more difficult parts here:
https://gerrit.wikimedia.org/r/391143 POC: Reimplementing MobileOptions in OOUI

5 was probably a safe estimate. I've left some questions up on the ticket.

Jdlrobson updated the task description. (Show Details)Nov 14 2017, 9:28 PM

Document the change of size in CSS and JS on this page somewhere under mw.org Reading/Web so that we have data about what the impact of including OOUI is on a given page.

Since no OOUI JS will be included on any page, is this still a useful task for CSS only?

Change 391143 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Reimplementing MobileOptions in OOUI

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

I finished exploring this and the resulting patchset details the remaining issues, but refraining from pulling into code review column until we have bandwidth to review. We should also take the time to read through and understand the issues before committing to continuing with this.

Jdlrobson claimed this task.EditedNov 28 2017, 6:22 PM

Patch is ready for review if anyone has bandwidth. Feel free to move back to "upcoming" if it creates noise. Patches are live on reading web staging.

Jdlrobson updated the task description. (Show Details)Nov 30 2017, 6:51 PM
pmiazga claimed this task.Nov 30 2017, 9:20 PM
pmiazga added a comment.EditedDec 4 2017, 11:49 PM

The form works nicely, but I'm being picky and I'm missing two things (not related to the form itself, more to the font resize feature itself):

  • when browsing the pages with different font size, first I see the "normal" font size, then it gets resized. That change is noticeable, would be nice to render it already resized
  • the font size doesn't apply to the mobile options form itself, when I change the font size I would like to see it applied also to the options page.
Jdlrobson updated the task description. (Show Details)Dec 5 2017, 12:28 AM

A few additional notes from Piotr other than the issues with the font-resize feature (maybe best discussed on T109364)

  • When in stable and opting into beta, you need to refresh page to see the other options. Is this something we want to fix?
  • Margin has increased slightly for pages without a tagline. Is this okay?

Note the patches are all merged for this task, but in a feature branch so we'll need to do this before testing/sign off (see "Sign off criteria" )

Jdlrobson reassigned this task from pmiazga to ABorbaWMF.Dec 5 2017, 12:30 AM

when browsing the pages with different font size, first I see the "normal" font size, then it gets resized. That change is noticeable, would be nice to render it already resized

The FOUC is unavoidable. we checked how popular websites do it and they have FOUC too. if there is a way to get rid of it, that would be great but if we can't, that's fine as well

the font size doesn't apply to the mobile options form itself, when I change the font size I would like to see it applied also to the options page.

changing font size of the control would be bit odd here. the buckets are such that they give an indication and it's not supposed to be a high fidelity control. we have nothing right now :) this is great to have for first version

When in stable and opting into beta, you need to refresh page to see the other options. Is this something we want to fix?

yeah I was going to surface this as well. let me answer this on T67079 we need to reflect the changes right away. right now the state is in flux.

Margin has increased slightly for pages without a tagline. Is this okay?
yes

Nirzar added a comment.EditedDec 5 2017, 12:38 AM

Just figured the font size for the feature title is wrong

form.mw-mf-settings .oo-ui-fieldLayout-header strong {
font-size:1em;
}

should be 1 not 1.1

Jdlrobson removed ABorbaWMF as the assignee of this task.Dec 5 2017, 12:41 AM
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: ABorbaWMF.

Jumped the gun here. Apparently some more changes needed ^ :)

Change 395166 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@specialpages] Tweak font size of special mobile options label

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

Change 395166 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Tweak font size of special mobile options label

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

Jdlrobson assigned this task to Nirzar.Dec 5 2017, 1:20 AM

@Nirzar not sure how we want to do qa on this. Do we want to defer till we deploy the code/feature branch. Are any further changes needed?

Nirzar added a comment.EditedDec 5 2017, 8:46 AM

can we deploy it on staging? I'm seeing checkboxes right now.

as far as the QA goes, do you think we should just finish the last piece https://phabricator.wikimedia.org/T67079 and then do QA for the entire settings + beta work? apart from that everything else seems to be working.

@ovasileva ??

Maybe we can do something like:

  • deploy to staging
  • set up QA task for @ABorbaWMF for testing the entire feature branch prior to deployment
  • skip QA for individual tasks but do a quick check from @ovasileva and @Nirzar during signoff

The changes were deployed to staging yesterday for this task. You can consider what's on staging as what would go out now.

I'm seeing checkboxes right now.

@Nirzar the toggle switch task (T169807)
is not in the current sprint and there's a few things to talk about before committing and estimating that.

as far as the QA goes, do you think we should just finish the last piece https://phabricator.wikimedia.org/T67079 and then do QA for the entire settings + beta work? apart from that everything else seems to be working.

What do you see as the final part? Showing the stuff in beta ? If so would be good to break that out into a clear subtask to avoid confusion.

Change 395606 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@specialpages] Default to regular font size in special mobileoptions

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

Change 395606 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Default to regular font size in special mobileoptions

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

When in stable and opting into beta, you need to refresh page to see the other options. Is this something we want to fix?

yeah I was going to surface this as well. let me answer this on T67079 we need to reflect the changes right away. right now the state is in flux.
Margin has increased slightly for pages without a tagline. Is this okay?
yes

So this is problematic due to the way we do feature management on mobile. We surface a config variable which is either true or false depending on whether the feature is enabled based on how the feature is configured.
Probably a 5 pointer to refactor and support that (unless we reload the page when the toggle switch is clicked which is a 1 pointer)

Change 396082 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@specialpages] Cookie must be set on server side

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

Prtksxna removed a subscriber: Prtksxna.Dec 8 2017, 9:42 AM

@Jdlrobson
can we just show/hide the lock>checkmark based on the toggle and save the preference async?

@Nirzar looking into this yesterday it seems we have to save the beta preference via a submission to the form. @bmansurov pointed out that the client side save was not working :( We could look into changing this to be synchronous but I think we'd be looking at a risky 8 pointer to do that. What do you think? Code on http://reading-web-staging.wmflabs.org/wiki/Special:MobileOptions reflects how it currently behaves without this work.

Change 396082 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Cookie must be set on server side

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

Nirzar closed this task as Resolved.Dec 21 2017, 7:45 PM

Macro votecat: looks good

👏🏽👏🏽👏🏽