Page MenuHomePhabricator

AMC page/talk toggle v1: Talk tab at the top of page for AMC users
Closed, ResolvedPublic8 Story Points

Description

Consider a feature branch to speed up development and encourage you to break this work into small progressive easily-reviewable chunks rather than one giant commit
To work out whether you are in AMC mode you will need to collaborate with whoever is working on T211197. You'll define the contract together given this is the first user, so think carefully!

This begins a set of changes to the talk page button which bring it to the top of the UI. For the purpose of this task, the changes here will result in 2 versions

  1. existing version (talk button at bottom of page)
  2. AMC version (talk button is displayed in a tab at the bottom of the page)

For the larger scope of the project and follow up changes please discuss on T210658: [EPIC] AMC Navigation - page/talk toggle.

Design notes

The treatment of the Article/Talk tabs will look like this:

The initial behavior of tapping on Talk will open the Talk overlay:

If the talk page doesn't exist, the Talk tab will be rendered in Base30, #72777d:

For colors, spacing, font-styling, and any other styling details please reference the prototypes:
Without Wikidata description: https://mobile-contributions.firebaseapp.com/nav4.html
With Wikidata description: https://mobile-contributions.firebaseapp.com/nav4-subtitle.html

QA instructions

Environment: staging
Browser & device: mobile (iOS/Safari, Android/Chrome), desktop (MacOS/Safari, Windows/Chrome)
Skin: mobile/default, desktop/Minerva
Other: try a few different languages (work with @Jdlrobson to enable other languages on staging)
Steps:

    • visit the following pages... trying them both in AMC mode AND stable mode. We are most worried about UI regressions so it would be useful to compare pages with en.wikipedia.org side by side.
  • Article page: visit an article page, evaluate relevant acceptance criteria, tap the "Talk" tab, tap "Read as wiki page" (link at bottom), tap on "Article"
    • User page: repeat steps from bullet above
    • Main page: evaluate relevant acceptance criteria
  • Special pages: Special:MobileOptions, Special:Contributions: check for UI regressions comparing to production.
  • CACHED html page: http://reading-web-staging.wmflabs.org/w/skins/MinervaNeue/Singapore.html - we are changing the HTML as part of this change so it's important we consider implications on pages cached in Varnish. Check for UI regressions comparing to production.
  • AMC mode should be available in some configurations and unavailable in others. An easy way to check if AMC is available is to go to Minerva's settings page and look for a big "advanced mobile contributions" toggle. If the toggle is present, AMC is available. If the toggle is not present, AMC is unavailable. When the toggle is on, AMC is active. When the toggle is off, AMC is inactive. Another check that AMC is both available AND active are the new talk tabs.
    • AMC is both available AND active when logged in, the AMC toggle is on, AND JavaScript is disabled (server) OR enabled (client) on the mobile Minerva skin.
    • AMC is both unavailable AND inactive (identical to today's production) when not logged in AND JavaScript is disabled (server) OR enabled (client) on both the mobile Minerva skin AND desktop Vector skin.
    • AMC is both available AND inactive when logged in, the AMC toggle is off, AND JavaScript is disabled (server) OR enabled (client) on the mobile Minerva skin.

Acceptance criteria

  • For any page that has a talk page, the navigation will display two tabs: primary and talk/discussion
    • For pages that have an empty talk page, the talk page tab will still be displayed (no red link)
  • The new talk tab will behave exactly like the existing "Talk" button at the bottom of the page. The new link should be a link to the talk page and open the talk overlay when clicked. ie. No changes will be made to the existing behaviour of the button
  • Naming - the text on each tab will be the text appearing on the desktop versions of the page based on project, language, and namespace.
    • For example, the WP namespace will have tabs that read “project page” and “talk”
  • For pages that do not have a talk page, no second tab will be displayed.
    • For example, Special:SpecialPages will only have the default tab “Special page”
  • Make sure Return to "Sandwich" page. is removed from the bottom of the Talk:Sandwich page for advanced mobile contribution users (now we have a tab at the top of the page this is redundant). It should continue to display in non-AMC mode.
  • the tabs will appear under wikidata descriptions
  • The new functionality is present for logged in enabled client Minerva, logged in enabled server Minerva; the old functionality is maintained for logged in disabled client Minerva, logged in disabled server Minerva, logged out client, logged out server.
  • For pages that do not exist, the landing page will be the main (article) page and the talk page tab will display as in all other cases above.
  • verify that Wikipedia:New_user_landing_page flow still works properly

Developer notes

  • The tabs in Vector are generated in SkinTemplate ::buildContentNavigationUrls (in core). It would be wise to use the urls generated here rather than generating our own so labels are consistent with desktop. An opportunistic refactor here might be helpful but not mandatory.
  • Those tabs needs to be rendered in PHP inside SkinMinerva/MinervaTemplate. Make sure to check the config flag to work out if you are in AMC mode
  • Remember to drop the talk button at the bottom of the page if you are in AMC mode.
  • For the JS talk overlay to continue to function - all that is needed is to ensure it is marked up with the class "talk" and has a data-title attribute (see https://github.com/wikimedia/mediawiki-skins-MinervaNeue/blob/b73f5c7520835cdbb3d0789f6201163a75fbefb9/includes/skins/user_page_links.mustache)

QA Results

ACStatusDetails))
1✅ PassedT212216#4983576
2✅ PassedT212216#4983576
3✅ PassedT212216#4983576
4✅ PassedT212216#4983576
5✅ PassedT212216#4986734
6✅ PassedT212216#4989565
7✅ PassedT212216#5008524
8✅ PassedT212216#4983576
9✅ PassedT212216#5008816

QA Results : Production

ACECARIDDetails))
1.1T212216#5047451
1.2T212216#5047451
2.1T212216#5047451
2.2T212216#5047451
3.1T212216#5047451
4T212216#5047451
5.1T212216#5047451
5.2T212216#5047451
6T212216#5047451
7.1T212216#5047451
7.2T212216#5047451
7.3T212216#5047451
8.1T212216#5047451
8.2T212216#5047451
9T212216#5047451

Event Timeline

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

Ok I feel like we're almost there. The tab underline is perfect on Android, but Safari is still one...pixel...off.

  1. On Safari the tab underline is rendering 1px too low. Here is a closeup:

  1. There is too much space between the title (or subtitle, depending on the wiki) and the new tabs:

@Jdlrobson recommended we split the remaining CSS work out into separate tasks so we can get this merged. Let me know if you want me to make one or two new tasks for the above items.

Change 489010 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] page actions is no longer position absolute

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

I'll follow up with @alexhollender on the remaining design issues when the initial patch is merged. After reviewing and merging please throw back to me in doing. I'll respond to initial patch feedback today.

Change 483637 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Talk tabs in AMC mode

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

Change 489011 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove backwards compatible CSS

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

Change 490396 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Talk tab select should sit on top of page actions

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

This has been design reviewed. As long as no code changes it can go straight to QA.

Change 490396 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Talk tab select should sit on top of page actions

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

Test Result

OS: macOS Mojave
Browser: Chrome DevTools Device Emulator (iPhone X)

Test Artifact(s):

AC1: ✅ PASS
For any page that has a talk page, the navigation will display two tabs: primary and talk/discussion
AMC Off:


AMC On:

AC2: ✅ PASS
For pages that have an empty talk page, the talk page tab will still be displayed (no red link)
AMC Off:


AMC On:

AC3: ✅ PASS
The new talk tab will behave exactly like the existing "Talk" button at the bottom of the page. The new link should be a link to the talk page and open the talk overlay when clicked. ie. No changes will be made to the existing behaviour of the button
AMC on/off with no Talk page:


AMC on/off with Talk page:

AC4: ✅ PASS
Naming - the text on each tab will be the text appearing on the desktop versions of the page based on project, language, and namespace.
For example, the WP namespace will have tabs that read “project page” and “talk”
For pages that do not have a talk page, no second tab will be displayed.
For example, Special:SpecialPages will only have the default tab “Special page”
Verified a number of pages. This artifact is representative of the sample:

AC5: ❌ FAIL
Make sure Return to "Sandwich" page. is removed from the bottom of https://en.m.wikipedia.org/wiki/Talk:Sandwich for advanced mobile contribution users (now we have a tab at the top of the page this is redundant). It should continue to display in non-AMC mode.
This was the same in staging at the URL above:

AC6: ⬜ Unable to test
the tabs will appear under wikidata descriptions
//Note: Could not find a page on staging with wikidata descriptions.

AC7: ⬜ Unable to test
The new functionality is present for logged in enabled client Minerva, logged in enabled server Minerva; the old functionality is maintained for logged in disabled client Minerva, logged in disabled server Minerva, logged out client, logged out server.

AC8: ✅ PASS
For pages that do not exist, the landing page will be the main (article) page and the talk page tab will display as in all other cases above.
AMC Off:


AMC On:

AC9:⬜ Unable to test
verify that Wikipedia:New_user_landing_page flow still works properly
//Need more info.

NOTE: Could not test Article as referenced in the QA steps. Could not find Article.
Edtadros updated the task description. (Show Details)Feb 26 2019, 5:34 AM
Edtadros updated the task description. (Show Details)Feb 26 2019, 5:37 AM
Edtadros reassigned this task from Edtadros to Jdlrobson.Feb 26 2019, 5:40 AM

@Jdlrobson please see my comments on the items that were Unable to test. There is one Fail that I didn't catch in my first pass until I delved into the Acceptance Criteria a bit more.

@Jdlrobson please see my comments on the items that were Unable to test. There is one Fail that I didn't catch in my first pass until I delved into the Acceptance Criteria a bit more.

Not sure why but the test link was on production. Either way, I was able to reproduce on staging:

Thanks @ovasileva. I was curious why the link was for production. I did test it in Staging as well but only made a reference to that above the image of prod.

Jdlrobson updated the task description. (Show Details)Feb 26 2019, 6:21 PM

Change 493089 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Return to link is not needed in AMC mode on talk pages

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

Above patch will drop the "Return to link" for AMC users.

Change 493089 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Return to link is not needed in AMC mode on talk pages

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

Jdlrobson reassigned this task from Jdlrobson to Edtadros.Feb 26 2019, 10:41 PM

Back to you! This should now be behaving correctly!

AC5: ✅ PASS
Make sure Return to "Sandwich" page. is removed from the bottom of the Talk:Sandwich page for advanced mobile contribution users (now we have a tab at the top of the page this is redundant). It should continue to display in non-AMC mode.

Non-AMC:

AMC:

Edtadros updated the task description. (Show Details)Feb 26 2019, 11:23 PM
Edtadros reassigned this task from Edtadros to Jdlrobson.Feb 26 2019, 11:28 PM

@Jdlrobson That fix worked. There are still 4 things I need more info/assistance to test: I have them listed as AC6, AC7, and AC9 (see T212216#4983576). Also, the QA steps reference testing an Article. I cannot find an Article in staging.

phuedx removed a subscriber: phuedx.Feb 27 2019, 12:02 PM

Test Result

OS: iOS 12.1.4
Browser: Safari
Device: iPhone XS Max

Test Artifact(s):*AC6:** ✅ PASS
the tabs will appear under wikidata descriptions

Edtadros updated the task description. (Show Details)Feb 27 2019, 9:28 PM
Edtadros added a subscriber: pmiazga.

@Jdlrobson Thanks for helping with AC6

@Niedzielski Can you help out with AC7: "The new functionality is present for logged in enabled client Minerva, logged in enabled server Minerva; the old functionality is maintained for logged in disabled client Minerva, logged in disabled server Minerva, logged out client, logged out server." I need some guidance on how to test this.

@pmiazga Can you help out with AC9 "verify that Wikipedia:New_user_landing_page flow still works properly"

@Edtadros, I've revised the QA steps

Edtadros added a comment.EditedMar 7 2019, 3:46 PM

=== Test Result

AC7

Status: ✅ PASS
OS: macOS Mojave
Browser: Chrome DevTools Device Emulator (iPhone X)

Test Artifact(s):
AMC is both available AND active when logged in, the AMC toggle is on, AND JavaScript is disabled (server) OR enabled (client) on the mobile Minerva skin.
JS Disabled:



JS Enabled:

AMC is both unavailable AND inactive (identical to today's production) when not logged in AND JavaScript is disabled (server) OR enabled (client) on both the mobile Minerva skin AND desktop Vector skin.
JS Disabled:


JS Enabled:




@Niedzielski Is this what is expected?

AMC is both available AND inactive when logged in, the AMC toggle is off, AND JavaScript is disabled (server) OR enabled (client) on the mobile Minerva skin.//**
JS Disabled:

JS Enabled:

AMC is both unavailable AND inactive (identical to today's production) when not logged in AND JavaScript is disabled (server) OR enabled (client) on both the mobile Minerva skin AND desktop Vector skin.
JS Enabled

This looks like a bug to me. Where is the discussion button?

Edtadros added a comment.EditedMar 7 2019, 5:21 PM

Test Result

Status: ✅ PASS
Worked with @pmiazga to test this.

Test Artifact(s):
AC9:verify that Wikipedia:New_user_landing_page flow still works properly*


Edtadros updated the task description. (Show Details)Mar 7 2019, 5:25 PM

@Niedzielski the Discussion button does show up when logged in. That screenshot was for the case where the user was logged in, AMC was disabled, and JS was enabled. Should this go back to Needs More Work?

@Edtadros, my mistake! Looks good to me @Edtadros

Edtadros reassigned this task from pmiazga to ovasileva.Mar 7 2019, 5:56 PM
Edtadros updated the task description. (Show Details)

It took some time...but everything is verified and passed! Thanks Everyone!

ovasileva closed this task as Resolved.Mar 11 2019, 9:49 AM

Thanks all for all your hard work on this. Talk tabs are done!

Edtadros added a comment.EditedMar 22 2019, 9:13 AM

Test Results: Production

OS: iOS
Browser: Chrome for iOS

Test Artifact(s):

AC1:
For any page that has a talk page, the navigation will display two tabs: primary and talk/discussion
AC1.1 AMC Off:

eswiki: ✅arwiki: ✅idwiki: ✅

AC1.2 AMC On:

eswiki: ✅arwiki: ✅idwiki: ✅

AC2:
For pages that have an empty talk page, the talk page tab will still be displayed (no red link)
AC2.1 AMC Off:

eswiki: ✅arwiki: ✅idwiki: ✅

AC2.2 AMC On:

eswiki: ✅arwiki: ✅idwiki: ✅

AC3:
The new talk tab will behave exactly like the existing "Talk" button at the bottom of the page. The new link should be a link to the talk page and open the talk overlay when clicked. ie. No changes will be made to the existing behaviour of the button
AC3.1 AMC on/off with no Talk page:

eswiki: ✅arwiki: ✅idwiki: ✅

AC3.2 AMC on/off with Talk page:

eswiki: ✅arwiki: ✅idwiki: ✅

AC4:
Naming - the text on each tab will be the text appearing on the desktop versions of the page based on project, language, and namespace.
For example, the WP namespace will have tabs that read “project page” and “talk”
For pages that do not have a talk page, no second tab will be displayed.
For example, Special:SpecialPages will only have the default tab “Special page”
Verified a number of pages. This artifact is representative of the sample:

eswiki: ✅arwiki: ✅idwiki: ✅

AC5:
AC5.1 Make sure Return to "Sandwich" page. is removed from the bottom of https://en.m.wikipedia.org/wiki/Talk:Sandwich for advanced mobile contribution users (now we have a tab at the top of the page this is redundant).

eswiki: ✅arwiki: ✅idwiki: ✅

AC5.2 It should continue to display in non-AMC mode.

eswiki: ✅arwiki: ✅idwiki: ✅

AC6:
the tabs will appear under wikidata descriptions

eswiki: ✅arwiki: ✅idwiki: ✅

AC7:
The new functionality is present for logged in enabled client Minerva, logged in enabled server Minerva; the old functionality is maintained for logged in disabled client Minerva, logged in disabled server Minerva, logged out client, logged out server.

AC7.1 Logged In with JS off

eswiki: ✅arwiki: ✅idwiki: ✅

AC7.2 Not Logged in with JS off

eswiki: ✅arwiki: ✅idwiki: ✅

AC7.3 Logged In with JS off and AMC ON

eswiki: ✅arwiki: ✅idwiki: ✅

AC8: ✅ PASS
For pages that do not exist, the landing page will be the main (article) page and the talk page tab will display as in all other cases above.
AC8.1 AMC Off:

eswiki: ✅arwiki: ✅idwiki: ✅

AC8.2 AMC On:

eswiki: ✅arwiki: ✅idwiki: ✅

AC9:⬜ Out of Scope
verify that Wikipedia:New_user_landing_page flow still works properly

eswiki: ⬜arwiki: ⬜idwiki: ⬜

This is not in scope for eswiki, arwiki, or idwiki.

Per @pmiazga:
that page is provided by ArticleCreationWorkflow extension
https://www.mediawiki.org/wiki/Extension:ArticleCreationWorkflow
and
per https://raw.githubusercontent.com/wikimedia/operations-mediawiki-config/master/wmf-config/InitialiseSettings.php it's used

'wmgUseArticleCreationWorkflow' => [
    'default' => false,
    'testwiki' => true,
    'test2wiki' => true, // T175302
    'enwiki' => true, // T192455
],
Edtadros updated the task description. (Show Details)Mar 22 2019, 9:15 AM
Edtadros updated the task description. (Show Details)Mar 22 2019, 9:19 AM
Edtadros updated the task description. (Show Details)Mar 22 2019, 11:55 PM
Edtadros updated the task description. (Show Details)Mar 23 2019, 12:07 AM

@Jdlrobson I get a very brief popup from the bottom, but it goes away. I've tried clicking it, it does nothing. Is it possible to get it in production?

Edtadros updated the task description. (Show Details)Mar 23 2019, 9:34 PM
Edtadros updated the task description. (Show Details)
Edtadros updated the task description. (Show Details)Mar 24 2019, 3:40 AM

@pmiazga please go to the very bottom of T212216#5047451 the only item remaining to test is AC9 for Arabic. You helped me test it before. It appears we are looking for a page with the discussion tab and project page tab, but I'm not entirely sure.

@pmiazga thanks for clarifying this. I have updated T212216#5047451 with the info you provided.

Edtadros updated the task description. (Show Details)Apr 4 2019, 1:03 AM