Page MenuHomePhabricator

Update the title alignment for special pages
Closed, ResolvedPublic3 Estimated Story Points

Description

As per the epic,
This is a first small change so we can move towards a more comprehensive special page template

Proposed change

  • Title alignment
  • Title size
  • Addition of a (optional) subtitle

Alignment

.pre-content.heading-holder h1 {
font-size: 24px;
margin-top:24px;
text-align: left;
}

Addition of an optional subtitle
<h1>Settings</h1>
<p class="mw-mf-page-subtitle">Reading Preferences</p>

  • This is an example of using these titles for Settings special page.
  • This subtitle is optional for individual special pages, it can or cannot exist

With Subtitle
https://zpl.io/2Z0GrM4

Without subtitle
https://zpl.io/bPq0xyA

Before

Simulator Screen Shot Jun 30, 2017, 2.57.02 PM.png (1×750 px, 109 KB)

After

Special Page title.png (1×750 px, 110 KB)

Developer notes

All mobile special pages are subclasses of MobileSpecialPage and have a shared ResourceLoader module defined in mobile.special.styles, thus a change to the settings page heading will impact all pages. It would be sensible to thus work in a feature branch and focus on the Special:MobileOptions page. The side effects of this change can then be dealt with in later cards.

Alignment change

Changing the alignment should require a straightforward and simple change to styling rules mobile.special.styles (

New tagline "reading preferences"

This part will be require minor code changes in both Minerva and MobileFrontend.

  1. SkinMinerva::getTaglineHtml should be relaxed so that it will also render a description (if available) on special pages

https://github.com/wikimedia/mediawiki-skins-MinervaNeue/blob/master/includes/skins/SkinMinerva.php#L793

  1. Inside the onSpecialPageBeforeExecute we can set a wikidata description using the special page name

https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/MobileFrontend.hooks.php#L573

$special->getOutput()->setProperty( 'wgMFDescription', 'Reading preferences' );

These two simple changes are all that needed.

AS part of this it might be tempting to rename MFDescription to MFTagline, but that should be considered going above and beyond!

Event Timeline

ovasileva set the point value for this task to 3.Nov 21 2017, 5:28 PM

While it's not ideal, MobileSpecialPage might allow us to colocate the generation of the HTML and the definition of the tagline. Consider the following example API:

class MobileSpecialPage {
  protected function getTitleText(): string {
    // ...
  }

  protected function hasTagline(): boolean {
    return $this->getTaglineText() !== null;
  }

  protected function getTaglineText(): string {
  }

  protected function getOutput(): string {
    // ...

   $html .= '<h1>' . $this->getTitleText() . '</h1>';

   if ( $this->hasTagline() ) {
    $html .= '<p class="mw-mf-page-subtitle">' . $this->getTaglineText() . '</p>';
   }
  }
}

class FooSpecialMobilePage extends SpecialMobilePage {
  protected function getTaglineText(): string {
    return $this->getMessage( 'foo-special-page-tagline' )
      ->getText();
  }
}

This might be preferable to transmitting taglines via properties on the output page. Thoughts?

Change 393881 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@specialpages] Show tagline on Special:MobileOptions page

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

Change 393920 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@specialpages] Render taglines on special pages and update heading styles

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

his might be preferable to transmitting taglines via properties on the output page. Thoughts?

My concern with this approach is it allows the tagline to be generated in two places. Both the special page and wikidata descriptions are styled the same - so conceptually I'd like to think of them as the same thing.

With the above approach Minerva would need to check if the special page is an instanceOf a special page when generating a tagline as well as checking the value of the wgMFDescription property on the OutputPage.
in https://gerrit.wikimedia.org/r/393881 I've pulled the logic for setting a description into one place - I'm hoping this will help us develop a common API that both pages can share. Happy to talk over this in person (maybe post-standup?)

@Nirzar - I've put the changes to Special:MobileOptions on staging. First pass exposes some issues with padding on Nearby. I'm working in a feature branch, so they will not go out to production until we are ready. We can plan these in a follow up (I'm guessing to fix T120520 and T169379). Let me know of any issues you find when testing that!

Happy to talk over this in person (maybe post-standup?)

I think I might be confusing how and where special page titles are generated. Let's chat! 👍

Just took a look at staging... looking good.

two minor things

  1. the extra upper margin of the title is missing
  2. the horizontal rule under the titles should be flush (edge to edge)

changes-tosettings.png (1×1 px, 201 KB)

for spec
zpl.io/bo0znYk


As far as the misalignment on nearby and page history. it should be fine for now.
we will clean those up piece by piece. i will create design debt cards for it. but they are not blockers for settings work

Is staging settings using oojs ui right now? the right alignment for checkboxes is looking good. we need to update the language for each setting though (description of beta for example) should i create cards for that or i can just add it here? they are just text changes.

The edge to edge is going to be a lot trickier so still thinking about how to do that. the problem is the page and input lines are full width but the padding needs to occur on each individual field in the form. The code of all special pages is quite closely coupled and since other pages don't behave that way it's making it tricker.

Top margin for heading should be easily addressed. Will fix that now.

Is staging settings using oojs ui right now?

Yes OOjs UI. I'll take a look at the text changes and comment on other card if I have any follow up questions.

Change 394116 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Render taglines on special pages and update heading styles

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

Change 394116 abandoned by Jdlrobson:
Render taglines on special pages and update heading styles

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

Change 394123 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@specialpages] Pages with only forms should take up entire page

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

@Nirzar Staging has been updated with all the latest changes.

Looking great. very minor tweak in the setting itself but I will comment on the other card.

as far as this card goes, this looks great to me! the nearby, login also looking good with the bottom padding to the title

Over to you devs!

If you could focus on the code I can fix any bugs you find in follow ups given I'm working on a feature branch. Am gonna squash all changes at end.

Change 394123 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@specialpages] Pages with only forms should take up entire page

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

Change 394435 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Set tagline on Special:MobileOptions page

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

Change 394435 abandoned by Jdlrobson:
Set tagline on Special:MobileOptions page

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

@Nirzar can you provide the design for tablet view? Everything looks nice and neat on mobile, but for tablets content will be centered with white margins on left and right. Because of that the gray bar dividing standard & beta options looks bit silly:

Screenshot from 2017-11-30 23-42-50.png (1×1 px, 46 KB)

Quick question before I render those mocks.

This card is about the title of settings page. we're discussing overall settings work here. don't have a problem with that, actually I think that's great but then I had few tweaks on css for actual settings as well. line-height for setting description "always expand all sections..." would be one example.

Should I give that feedback here on this card?

Now about the iPad

Here's the easy way, where the grey bar which is kinda the gap between two white panels exposing the background base color. it's more of showing the base layer and the white panels are laid on top of it. it's not really a bar, it's a gap.

Fig 1

easy.png (1×2 px, 215 KB)

so the ideal version would be something like this ->
Fig 2

ideal.png (1×2 px, 216 KB)

but this will require a lot of change in how we lay out our content. I'm taking care of all this in Marvin design system. for now let's just extend the bar like shown in Fig 1. we can't really fix the underlying issue of how we lay our content.

css changes

Line height for description should be 1.4em instead of 2em

form.mw-mf-settings .option-description {
line-height:1.4em;
}

Margin top for tagline should be 2px instead of 4px, apologies if zeplin doesn't say this value as 2.

.heading-holder .tagline {
margin:2px 0 0;
}

Copy changes

Increase or decrease the size of the text for readability ==> "Adjust article font size for better readability"

Always expand all sections when navigating to a new page ==> "Always expand sections of all articles by default"

By joining the beta, you will get access to the latest experimental features which may be subject to software defects. ==> "Join %project% Beta to get access to the latest experimental features!"

we will inform the user about defects in a tagline inside beta

Change 394633 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@specialpages] Optimise Special:MobileOptions for tablet display

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

Change 395140 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@specialpages] Center content-header for history/watchlist

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

Change 395151 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@specialpages] Use OOUI\HiddenInputWidget

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

Change 393881 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Set tagline on Special:MobileOptions page

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

Change 395151 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@specialpages] Use OOUI\HiddenInputWidget

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

Change 393920 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@specialpages] Always render taglines unconditionally and update styles

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

Jdlrobson added a subscriber: pmiazga.

Several patches remain:

@Nirzar - not sure if the 2nd patch is needed. Let us know if we should remove the centering of content under the heading
With patch:

Screen Shot 2017-12-04 at 4.35.32 PM.png (429×1 px, 40 KB)

Screen Shot 2017-12-04 at 4.36.13 PM.png (337×1 px, 28 KB)

Without:
Screen Shot 2017-12-04 at 4.39.48 PM.png (411×1 px, 32 KB)

Screen Shot 2017-12-04 at 4.40.03 PM.png (442×1 px, 43 KB)

let it be center aligned for now

Change 395140 abandoned by Jdlrobson:
Center content-header for history/watchlist

Reason:
Per https://phabricator.wikimedia.org/T180095#3811281

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

Per standup, https://gerrit.wikimedia.org/r/#/c/394633 is the only remaining task. When done, this can be signed off alongside the other settings task.

Still todo:

  • it's not possible to submit the form if javascript is disabled
  • "Save" button placement is a bit off (without javascript)

There is one issue with font resizer:
Fonts get adjusted even beta mode is off.
Steps to reproduce: go to mobile options, enable beta mode, change the font, disable beta options.
Expected: Articles will be displayed used regular font size

Change 395644 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@specialpages] Correct alignment of save button in Special:MobileOptions

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

it's not possible to submit the form if javascript is disabled

I can't replicate this issue. Works fine for me.

Fonts get adjusted even beta mode is off.

font resizer has always worked this way. It's being promoted to stable so I wouldn't worry too much about this.

"Save" button placement is a bit off (without javascript)

Fixed in https://gerrit.wikimedia.org/r/395644

Change 394633 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@specialpages] Optimise Special:MobileOptions for tablet display

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

Change 395644 abandoned by Jdlrobson:
Correct alignment of save button in Special:MobileOptions

Reason:
whoops. Guess I folded it into the parent commit in my rebase. Guess this is done then :)

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

All changes are now up on staging.

@Nirzar - over to you for review. I'll sign them off afterwards.