Page MenuHomePhabricator

Style the history page for AMC users
Closed, ResolvedPublic5 Story Points

Description

We will style the history page given Alex's mocks below.

Design

enes (approximation)

Acceptance criteria

  • The page looks like Alex's mocks!
  • new styles are scoped to AMC mode
  • The new styles are defined as a skinStyle on mediawiki.action.history.styles.less

Developer notes

https://gerrit.wikimedia.org/r/500133 will provide you with the boilerplate to get started - it resets all the history styles that are not needed from desktop. All styles can be shipped in mediawiki.action.history.styles.less to avoid having to worry about other special pages.

There is one gotcha to be aware of in estimation:

  • The HTML can contain invisible characters e.g. ‎ that can interrupt display block and inline block and cannot be hidden via CSS. You may need to use inline-block where block makes sense

Scoping these styles to AMC would be useful as it allows us to remove the mobile special history page in future.

Changes to be made

  • hide blurb of instructional text at top of page
  • ditch gray background highlighting - given our space constraints I don't think it's worth it, since in order to have a bg color you need to add margins so it doesn't look super weird
  • set font-size for "Undo" button to 0.75em
  • fix z-index of sticky "Compare selected revisions" button so it is behind the dark overlay that appears when opening the main menu
  • use the Wikimedia style guide red (#DD3333) and green (#00AF89) for the byte change stuff
  • fix radio button bug - radio buttons don't hide/show correctly
  • form at top of page should be collapsed by default
  • figure out what to do with "Change visibility of selected revisions" + "Select: All, None, Invert" buttons
  • change class for "Compare selected revisions" button to mw-ui-button mw-ui-primary mw-ui-progressive
  • remove border-radius: 5px for "Compare selected revisions" button
  • remove height: 50px for "Compare selected revisions" button
  • remove button-y styles from "Undo" button and instead use .mw-ui-button

QA Criteria

  • For logged-in user with AMC mode turned on, navigating to a history page will lead to the "action=history" page instead of "Special:History" page.
  • The action=history page should resemble the mock provided in this task description.
  • The diff button, "compare selected revisions" should work when selecting two revisions.

A Special history URL looks like this: https://en.m.wikipedia.org/wiki/Special:History/Banana
An action=history URL looks like this: https://en.wikipedia.org/w/index.php?title=Banana&action=history

Noting that the following test cases from T215597: QA edit tags for moderation actions will be tested as a part of this task:

  • AC1: undo: tags should show up in user contributions
  • AC2: rollback: tags should show up in user contributions

Sign off steps

  • Make sure regression T222394 has been resolved.

QA Results

Event Timeline

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

@Jdrewniak here's what I'm seeing currently on iPhone and Android:

iPhone
Android

Main things I'm noticing:

  • radio buttons on iOS are weird
  • general alignment stuff
  • location of undo button

Let me know if it makes sense to chat, or if you'd rather work on it a bit more first. Also here are some links of pages with history on staging:
https://reading-web-staging.wmflabs.org/wiki/Antonio_Vivarini
http://reading-web-staging.wmflabs.org/w/index.php?title=Nothing_to_Hide_(book)&action=history
https://reading-web-staging.wmflabs.org/wiki/Deutsches_Stadion
https://reading-web-staging.wmflabs.org/wiki/Changampuzha_Krishna_Pillai

Just a note on the radio buttons. That effect on iOS is actually due to some overly broad rules in the Minerva reset.less file:

input {
border: 0; 
background: none;
}

It looks like @Volker_E and @Jdlrobson have already removed this rule in a feature branch. Once that branch is merged, the radio buttons should look "normal" on iOS. Until then though, I'll add a custom background-colour and border to those inputs. It looks like MacOS ignores those input styles, but I'm not sure what effect they'll have on other browsers yet.

Looking sweet. Here are screenshots of what I'm seeing:

normaladmin
iOS
Android

Few things I'm wondering about:

  1. there seems to be a bug with the radio button functionality...both radio buttons for every entry seem to be visible, whereas in production you would only see both available for entries that are in-between the top selection and the bottom selection. You're also able to select both radio buttons of a single entry at once.
  2. will the form at the top ("Filter revisions") be collapsed by default? (on desktop/Vector it is, which was a recent change)
  3. do you think we should just hide the blurb of instructional text below the filter box? @Jdlrobson
  4. what should we do about these elements below the "Compare selected revisions" buttons in admin mode? They will be present on some other Wikis (e.g. ES) even for non-admins. I feel like they're not important enough to be in the sticky header, though it looks like it would require some additional work in the core HTML to be able to leave them out of the sticky header.
  5. seems like something got mixed up with the gray background highlighting — it's intended to work such that the two edits you have selected for comparison (via radio buttons) have gray backgrounds, though now it seems to be applied as a hover state: .minerva--amc-enabled.action-history #pagehistory li:hover { background: #eaecf0; }. Thoughts on that? On desktop a .selected class gets added when you select the radio button for a row, and then the background is tied to that, but I don't see that class on mobile.
  6. should we ditch the "Compare selected revisions" button at the bottom of the page?
  7. how can we test this in other languages? I mentioned this earlier on, though not to you. Hopefully it doesn't feel like a crazy ask. I think it's more or less necessary to do for Indonesian, Spanish, and Arabic, as well as a for maybe the top 5 wikis

@alexhollender re: radio button functionality & grey background: Yeah, it looks like the Javascript that's responsible for changing the visibility of the radio buttons, as well as the background color of the selected revision, isn't being run on mobile. I'm not sure if there's an easy way to enable this, but I'll look into it.

@alexhollender

should we ditch the "Compare selected revisions" button at the bottom of the page?

what should we do about these elements below the "Compare selected revisions" buttons in admin mode?

sure :)

@alexhollender re: radio button functionality & grey background: Yeah, it looks like the Javascript that's responsible for changing the visibility of the radio buttons, as well as the background color of the selected revision, isn't being run on mobile. I'm not sure if there's an easy way to enable this, but I'll look into it.

This looks like an oversight on my part and should be easily fixed. Ping me today and we can get this turned on.

@alexhollender I'm also wondering if there should be a max-width for the primary button. (I think there should be one)

More to the button treatment/interface:

  • it should be a mw-ui-button mw-ui-primary mw-ui-progressive. Why are we using mediawiki.ui again?
  • 5px border-radius >> 2px
  • 50px height >> remove?!
  • Revert button should look like a button, adding class .mw-ui-button (also see above)?
  • :hover effect of rows should get a transition

Should we turn all of those issues into check list in task description?

Change 508415 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/core@master] Adding mobile/desktop targets to mediawiki.action.history module

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

@Volker_E thanks so much for taking a close look at this. Here is what the page looks like with your changes:

Would it be okay to define font-size: 12px for the Undo button? We are working with unusual space constraints here. Here's how it looks (I also added a text-transform: capitalize):

@Jdrewniak all of the changes @Volker_E listed in T219895#5161495 look good to me. I will add a running list of updates needed to the task description. Feel free to cross them off as you make the respective changes.

Regarding my comment:

  1. what should we do about these elements below the "Compare selected revisions" buttons in admin mode? They will be present on some other Wikis (e.g. ES) even for non-admins. I feel like they're not important enough to be in the sticky header, though it looks like it would require some additional work in the core HTML to be able to leave them out of the sticky header.

I'm not sure if we can get away with putting them below the list. I think in reality the list is quite long. Is there any way to leave them at the top, but exclude them from the sticky header?

Volker_E added a comment.EditedMay 6 2019, 9:42 PM

As long as it's a 12px em equivalent, sure. Defining px for font-size or line-height is an accessibility no-go as users can't override them when setting their own font size preferences in the browser – a commonly used override feature.
Also from an internationalization standpoint, using text-transform is not recommended.

Change 508415 merged by jenkins-bot:
[mediawiki/core@master] Adding mobile/desktop targets to mediawiki.action.history module

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

alexhollender updated the task description. (Show Details)May 6 2019, 9:52 PM

Change 508451 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/core@master] Moving button mixin into separate mediawiki.mixins.buttons.less file

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

@Volker_E, for this task, we don't want to change the look these buttons on desktop, so we're not placing the css classes mw-ui-button mw-ui-primary mw-ui-progressive in the button HTML. Instead, we have to add these styles to the existing classes. To achieve this, we have to create a mixin that contains all the button styles we want to use on this element. The core patch above moves one button mixin into the mediawiki.less folder, making it available in Minerva.

Volker_E added a comment.EditedMay 7 2019, 8:59 AM

@Jdrewniak Makes sense to not change the HTML classes!
I'm not convinced by the mixin change though, as you can call the mediawiki.ui mixin from whereever. Changing the mixin to a different MW core button mixin buries the fact, that mediawiki.ui is deprecated and should be replaced by OOUI.

you can call the mediawiki.ui mixin from whereever

Unfortunately that's not the case :( Only files in resources/src/mediawiki.less/ get that global magic, whereas the button styles are in resources/src/mediawiki.ui/.

I could move the mixin to resources/src/mediawiki.less/mediawiki.ui/ then it could be imported as
@import 'mediawiki.ui/mixins.buttons';
instead of
@import 'mediawiki.mixins.buttons';

Might make it clear that it's part of mediawiki-ui.

p.s. if I could import the OOUI button styles into this "SkinStyles" stylesheet I would, but it looks impossible to import anything other than the files in mediawiki.less/ from there.

Volker_E edited subscribers, added: Krinkle; removed: Tbayer.May 7 2019, 11:40 AM
alexhollender updated the task description. (Show Details)May 7 2019, 6:26 PM
alexhollender updated the task description. (Show Details)

Change 508451 merged by jenkins-bot:
[mediawiki/core@master] Making mediawiki.ui button mixin available for importing globally.

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

alexhollender updated the task description. (Show Details)May 7 2019, 6:42 PM
ovasileva updated the task description. (Show Details)May 8 2019, 10:13 AM

Change 508811 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/core@master] Extract default mediawiki-ui button styles into mixins

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

Change 505769 had a related patch set uploaded (by Jdlrobson; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Adds AMC styles for action=history page

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

Krinkle removed a subscriber: Krinkle.May 8 2019, 3:43 PM

Change 508811 merged by jenkins-bot:
[mediawiki/core@master] Extract default mediawiki.ui button styles into mixins

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

cmadeo added a subscriber: cmadeo.May 10 2019, 5:29 PM
alexhollender updated the task description. (Show Details)

Change 510117 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Add a LESS z-index variable for elements that sit above page content

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

Change 510117 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Add a LESS z-index variable for elements that sit above page content

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

Change 505769 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Adds AMC styles for action=history page

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

Change 512692 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Fix z-index issue with button on action=history page

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

Jdrewniak added a comment.EditedMay 27 2019, 3:10 PM

The patch above fixes the z-index issue.

For the green/red color change however, I'm going to need some guidance from @Volker_E. If I want to use a variable from mw-ui-base (@wmui-color-green30 specifically) in Mediawiki core, how would I do that?

Specifically, I want line#16 in resources/src/mediawiki.special.changeslist/default.less to read:
color: @wmui-color-green30;

  • Should I import the mw-ui-base.less file directly? like @import "../../resources/lib/ooui/wikimedia-ui-base.less"?
  • Create a resourceLoader module with just that file and declare it a dependency for the mediawiki.special.changeslist module?
  • Declare OOUI as a dependency for the mediawiki.special.changeslist module?
  • Redeclare this value as a variable in mediawiki.less/variables.less so it's globally available?
  • Just hard-code the value?

I hope this is easier than the button mixin :P

The use of central variables to make life's like yours easier got stuck in T123359. Myself and @Krinkle agreed to continue with his architectural insights, but other priorities stopped us on the way.

Jdrewniak updated the task description. (Show Details)May 30 2019, 2:58 PM
Jdrewniak updated the task description. (Show Details)May 30 2019, 3:40 PM

Change 512692 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Fix z-index issue with button on action=history page

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

Edtadros added a subscriber: Edtadros.

Test Result

Status: ⬜ Not Complete, Need More Info
OS: macOS Mojave
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):

QA Criteria

✅ AC1: For logged-in user with AMC mode turned on, navigating to a history page will lead to the "action=history" page instead of "Special:History" page.

❌ AC2: The action=history page should resemble the mock provided in this task description.
The radio buttons do not appear the same as in the description image. In addition, they appear to change depending on what I select.

✅ AC3: The diff button, "compare selected revisions" should work when selecting two revisions.

✅ AC4:undo: tags should show up in user contributions

❓ AC5: rollback: tags should show up in user contributions
@Jdrewniak, I cannot figure out how to do a rollback to verify if the rollback tag appears.

Edtadros updated the task description. (Show Details)May 31 2019, 1:34 PM

❌ AC2: The action=history page should resemble the mock provided in this task description.
The radio buttons do not appear the same as in the description image. In addition, they appear to change depending on what I select.

@Edtadros good catch. This is an issue with the mock. Please ignore the visual difference here. My apologies.

ovasileva updated the task description. (Show Details)Jun 4 2019, 9:41 AM
ovasileva updated the task description. (Show Details)Jun 4 2019, 9:44 AM
ovasileva added a comment.EditedJun 4 2019, 9:49 AM

❓ AC5: rollback: tags should show up in user contributions
@Jdrewniak, I cannot figure out how to do a rollback to verify if the rollback tag appears.

@Edtadros - there was a small issue here - from mobile, it would be easier to see a rollback on the recent changes page or on the desktop version of user contributions - I'll edit the criteria to specify. To get rollback rights, you must be logged in as an admin. I tested this quickly and will mark this AC as done:
rollback button appears:


revert shows in user contributions:

rollback tag appears in recent changes:

ovasileva updated the task description. (Show Details)Jun 4 2019, 9:50 AM

❌ AC2: The action=history page should resemble the mock provided in this task description.
The radio buttons do not appear the same as in the description image. In addition, they appear to change depending on what I select.

@Edtadros good catch. This is an issue with the mock. Please ignore the visual difference here. My apologies.

Marking this as complete as it is showing expected behavior

ovasileva updated the task description. (Show Details)Jun 4 2019, 9:50 AM

@Edtadros - all AC pass. If you don't have any concerns, I think we can go ahead and move this to signoff

Edtadros reassigned this task from Jdrewniak to ovasileva.Jun 4 2019, 3:02 PM

@ovasileva everything looks good. Thanks for your help/clarification.

ovasileva updated the task description. (Show Details)Jun 4 2019, 3:27 PM
ovasileva closed this task as Resolved.Jun 4 2019, 5:02 PM

In that case, we're done! Thanks all.

Change 514449 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[operations/mediawiki-config@master] Enable the new history page in the advanced mobile contributions mode

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

Change 514449 merged by jenkins-bot:
[operations/mediawiki-config@master] Enable the new history page in the advanced mobile contributions mode

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

Mentioned in SAL (#wikimedia-operations) [2019-06-05T11:43:41Z] <pmiazga@deploy1001> Synchronized wmf-config/InitialiseSettings.php: SWAT: [[gerrit:514449|Enable the new history page in the advanced mobile contributions mode (T219895)]] (duration: 00m 56s)