Page MenuHomePhabricator

Add History to article toolbar for all logged-in users
Closed, ResolvedPublic2 Estimate Story Points

Description

Description

The History icon will appear in the article toolbar for all logged-in users.

Acceptance criteria

  • wgMinervaHistoryInPageActions should only apply to logged in users. Make sure this is true!
  • Tapping on the History icon will lead (logged-in, non-AMC) users to the simplified mobile version of History.
  • The default in Minerva should match the deployed version. Update skin.json after the deploy has been successful.

Designs

AndroidiOS

Developer notes

There is currently no flag for "logged in mode". Should there be?

Using the existing implementation we could make

wgMinervaHistoryInPageActions['base'] = true

and add a check for whether the user is logged in or not in the logic.

A later task can remove wgMinervaHistoryInPageActions altogether

QA Steps

  1. go to beta cluster as anon user
  2. go to any article page and check that the history icon is not visible in the toolbar
  3. log in as test user
  4. redo step two, but this time verify that history icon exists
  5. select history icon and verify that the mobile version of the history page opens
  6. redo step two with AMC on, verify that history icon exists
  7. select history icon and verify that the expanded version of the history page opens

QA results

Details

Related Gerrit Patches:
mediawiki/skins/MinervaNeue : masterAdd History icon and Article/Talk tabs to default loggedin experience
operations/mediawiki-config : masterAdd History to article toolbar for all logged-in users
mediawiki/extensions/MobileFrontend : masterIntroduce new 'loggedin' mode
mediawiki/extensions/MobileFrontend : masterProvide an AudienceMode to limit certain features to given audience

Event Timeline

ovasileva triaged this task as Medium priority.Sep 12 2019, 1:16 PM

Where does history link to? The AMC version or the current mobile simplified version?

Jdlrobson updated the task description. (Show Details)Sep 12 2019, 4:42 PM

Where does history link to? The AMC version or the current mobile simplified version?

Simplified version. I updated the task description.

@Alex I just want to confirm - those mocks are not in sync, right? We do not show a different toolbar for Android and iOS.

The Android mock shows the mobile mode in the AMC mode without overflow menu (icons are spread through the width of toolbar - same margins for all icons)
The iOS mock shows the mobile mode in regular mode - language icon is on left, all icons goes to the right.

@Alex I just want to confirm - those mocks are not in sync, right? We do not show a different toolbar for Android and iOS.
The Android mock shows the mobile mode in the AMC mode without overflow menu (icons are spread through the width of toolbar - same margins for all icons)
The iOS mock shows the mobile mode in regular mode - language icon is on left, all icons goes to the right.

On Android the download action appears, which means there are 5 icons in the toolbar and they should be spaced out evenly. On iOS there are only 4 icons in the toolbar. Is it possible for us to do this layout?

@alexhollender everything is possible, but that's probably a new task. Also, please note, that the DownloadIcon is injected via JS, thus it may cause FOUC when we decide to change styling based on the number of icons in the toolbar.

How DownloadIcon works - after we render the page, the JS that tests browser printing ability. When the print feature is available system will inject Download Icon.

Jdlrobson updated the task description. (Show Details)Sep 24 2019, 4:21 PM
Jdlrobson updated the task description. (Show Details)Sep 24 2019, 4:25 PM
ovasileva set the point value for this task to 2.Sep 24 2019, 4:25 PM
nray added a subscriber: nray.Sep 24 2019, 4:26 PM

@pmiazga Would it be wise to add another user mode under includes/features in MF to account for logged in users?

Change 540378 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@master] Introduce new 'loggedin' mode

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

@nray yup, that's what I did.

Restricted Application added a subscriber: Masumrezarock100. · View Herald TranscriptNov 6 2019, 9:55 AM

Change 549190 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@master] Provide an AudienceMode to limit certain features to given audience

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

ovasileva raised the priority of this task from Medium to High.Dec 10 2019, 1:33 PM

Change 549190 abandoned by Pmiazga:
Provide an AudienceMode to limit certain features to given audience

Reason:
it was POC, we don't need it now, there is no need to go that direction yet

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

Change 540378 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Introduce new 'loggedin' mode

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

Change 556440 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[operations/mediawiki-config@master] Add History to article toolbar for all logged-in users

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

Jdlrobson assigned this task to pmiazga.Dec 13 2019, 7:46 PM

Change 556440 abandoned by Pmiazga:
Add History to article toolbar for all logged-in users

Reason:
decided to update the skin.json version in repo to keep experience for 3rd parties consistent with prod config.

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

Change 557100 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/skins/MinervaNeue@master] Add History icon and Article/Talk tabs to default loggedin experience

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

Change 557100 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Add History icon and Article/Talk tabs to default loggedin experience

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

pmiazga reassigned this task from pmiazga to Edtadros.Dec 16 2019, 7:52 PM
pmiazga updated the task description. (Show Details)
ovasileva added a comment.EditedDec 17 2019, 10:48 AM

QA (beta cluster) - passed

tested on: chrome desktop 79.0.3945.79, chrome mobile 78.0.3904.108 Android 9

  1. go to beta cluster as anon user
  2. go to any article page and check that the history icon is not visible in the toolbar


  1. log in as test user
  2. redo step two, but this time verify that history icon exists


  1. select history icon and verify that the mobile version of the history page opens

  1. redo step two with AMC on, verify that history icon exists


  1. select history icon and verify that the expanded version of the history page opens

ovasileva updated the task description. (Show Details)Dec 17 2019, 10:50 AM
ovasileva updated the task description. (Show Details)Dec 17 2019, 10:54 AM
ovasileva updated the task description. (Show Details)
ovasileva updated the task description. (Show Details)Dec 17 2019, 10:56 AM

QA (Wikimedia) - ✅Passed

tested on: chrome desktop 79.0.3945.79, iPhone X

  1. go to beta cluster as anon user

✅2. go to any article page and check that the history icon is not visible in the toolbar

  1. log in as test user

✅4. redo step two, but this time verify that history icon exists

✅5. select history icon and verify that the mobile version of the history page opens

✅6. redo step two with AMC on, verify that history icon exists

✅7. select history icon and verify that the expanded version of the history page opens

Edtadros updated the task description. (Show Details)Dec 18 2019, 6:41 AM

@Edtadros - were you able to test on mediawiki? The screenshots above seem to be from the beta cluster.

Verified on hewiki:
logged out:


amc:

  • history link leads to special history page

logged-in:

  • history link leads to mobile history
Edtadros updated the task description. (Show Details)Dec 19 2019, 8:19 PM
Edtadros reassigned this task from Edtadros to ovasileva.Dec 19 2019, 8:33 PM
Edtadros added a subscriber: Edtadros.

QA (Wikimedia) - ✅Passed

tested on: chrome desktop 79.0.3945.79, iPhone X

  1. go to beta cluster as anon user

✅2. go to any article page and check that the history icon is not visible in the toolbar

  1. log in as test user

✅4. redo step two, but this time verify that history icon exists

✅5. select history icon and verify that the mobile version of the history page opens

✅6. redo step two with AMC on, verify that history icon exists

✅7. select history icon and verify that the expanded version of the history page opens

Edtadros updated the task description. (Show Details)Dec 19 2019, 8:33 PM
ovasileva closed this task as Resolved.Jan 6 2020, 11:50 AM

all done!

alexhollender added a comment.EditedJan 6 2020, 4:28 PM

There seems to be one issue here (only affecting iOS) — not sure if we want to re-open this task or create a new one?

For logged-in, non-AMC users the icons in the toolbar are incorrectly spaced:

currentdesign
mobile
desktop

Could be related to T240868, though that issue seems to have gone away.

I think the challenge here is the download icon is conditional. It would be great to fix that (we have the print service after all.. we're just not using it) or push the overflow menu to all users and leave the download icon in there. Fixing T201954 would remove the need for knowing about, maintaining and testing 2 different page action layouts and would be my recommendation here.

Is the layout broken on Chrome with JS disabled as well?

Ah right, I forgot to check desktop. I've updated my comment above to include screenshots from desktop.

Is the layout broken on Chrome with JS disabled as well?

Yes, here is Chrome on Android with JS disabled: