Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Page MenuHomePhabricator

Render talk page as a tab (rather than a modal)
Closed, ResolvedPublic5 Estimated Story Points

Description

Description

In order to allow users to switch between Talk and Article views more fluidly, and also to match the expectations of the tab pattern, we should render the talk page as a tab rather than in a modal. This is a pre-requisite for enabling the Article/Talk tabs for all users. Our expectation is that this is the first task and a blocker for the remainder of this process, with the actual switch happening later in Q2, after AMC mode is deployed everywhere and sufficient outreach has been completed. While the initial group receiving these changes will be AMC users, the target for this is all logged-in users. As such, this task is not a part of the AMC project.

Acceptance criteria

Note: See developer notes about how we cannot do this in PHP. This is a CSS/JS only change

  • The talk page is feature flagged behind "TALK_AT_TOP" config (both Talk At Top and new Talk Page will use same Feature). (This AC turned out not to be relevant as the simplified talk page is shown whether amc is enabled or not)
  • When I click the talk tab we navigate to the Talk: page via a server side render.
  • When I click a section heading a TalkSectionOverlay renders
  • When I disable JavaScript I see the entire talk page with edit icons for editing individual sections
  • When I click "view as wiki page" I see the entire talk page with edit icons for editing individual sections
  • The #/talk and #/talk/1 routes have been removed. The latter is replaced with an overlay that shows when I navigate to '#Talk_Anchor'
  • The talkBoard code has been moved from MobileFrontend (it is no longer needed because of the above A/C)
  • When I click the "add new topic" I navigate to "#/talk/new" and the talk overlay for creating topics renders as it does currently
  • Given the talk section content is inside the page, we can remove the code that hits the mobileview API and make the TalkSectionOverlay a synchronous component.
  • The CSS for the talk page should be restricted to talk pages only - see skins.minerva.userpage.styles for how to do this.

Designs

article viewtalk viewdiscussion view (modal)add new discussion view (modal)
image.png (1×750 px, 346 KB)
image.png (1×750 px, 83 KB)
en.m.wikipedia.org_wiki_Akira_Kurosawa(iPhone 6_7_8) (2).png (1×750 px, 198 KB)
en.m.wikipedia.org_wiki_Ky%C5%ABjitai(iPhone 6_7_8).png (1×750 px, 65 KB)

Note: once you've navigated to the Talk tab the actions in the toolbar (language, watch, history) and overflow menu should apply to the Talk page, not the Article page. For example tapping History would lead to the history of the Talk page (e.g. https://en.wikipedia.org/w/index.php?title=Talk:China&action=history). Let's be mindful to consider and test this. I know that Watch is the same regardless of whether you are on the Article or Talk tab.

Developer notes

We can do this using CSS and JS alone.
I have a patch which proves this:
https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/540441 POC: Talk is a tab

Sign off steps

  • The code for mobile.talk.overlays is now so small it should be folded into mobile.startup. Set up a task to do that.
  • Document the change in bytes in this ticket.
  • Confirm T237155 is resolved. If not pull it into the sprint to fix.

QA steps

When anon user visits talk page directly

  1. Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Dog as an anon user
  2. Ensure that an "Add discussion" button is NOT displayed
  3. Ensure there is a "Read as wiki page" button at bottom of page
  4. Ensure that clicking on a section opens up an overlay where you can see the topic title, comment, and any replies to that topic. You won't be able to add a reply since you are an anon,.
  5. Click the "Read as wiki page" button. Ensure that it lets you see the page with a Table of Contents (on wide screens), but without the "Read as wikipage" button or the "Active discussions" header.
  6. Click on a link in the TOC and ensure it leads you to the top of clicked section
  7. Ensure that you can collapse/expand a section by clicking on it and that no overlay shows
  8. Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:History/Talk:Main_Page?action=history and ensure that you don't see an "Add discussion button" or a "Read as wiki page" button

When logged in, non-AMC user visits article

  1. Login and visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Dog
  2. Ensure that there is a "Discussion" button at bottom of page. Click on it and ensure you are taken to the talk page (no overlay appears).
  3. Ensure there is a "Read as wiki page" button at bottom of page, "Add discussion" button at top of page, and "Active discussions" heading with a list of topics below that
  4. Ensure that clicking on a section opens up an overlay where you can see the topic title, comment, and any replies to that topic. Type a reply into the overlay and click the "Publish" button. Ensure that the page refreshes and the overlay appears again with the reply you typed visible (overlay should open again automatically). Exit overlay.
  5. Click the "Add discussion" button and ensure an overlay appears that lets you add a discussion.
  6. In the "Subject" field, type a subject with ascii and non-ascii characters e.g. ("😭\o/abc123!*"'();:@&=+$,/?#[]%F0%9F%98%AD存在%😭") should cover most of the edge cases out there. In the "What is on your mind?" field, type whatever you want. When done, click the "Publish" button. Ensure that the page refreshes and a toast appears saying your save was successful. Scroll to bottom of page and ensure your topic appears in the list. Click on the topic and ensure the overlay appears and shows what you wanted to publish.
  7. Click the "Read as wiki page" button. Ensure that it lets you see the page with a Table of Contents (on wide screens), but without the "Read as wikipage" button and without the "Active discussions" header.
  8. Click on the topic you added in step 6 in the TOC and ensure it leads you to the top of the section.
  9. Ensure that you can collapse/expand a section by clicking on it and that no overlay shows
  10. Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Main_Page?action=history and ensure that you don't see an "Add discussion button" or a "Read as wiki page" button

When logged in user visits a talk page with Flow enabled

  1. Login and visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Flow_QA
  2. Ensure that there is NOT a Table of contents, "Add discussion" button, "Read as wikipage" button, "There are no discussions on this page" message, or "Active discussions" message visible on the page.
  3. Ensure that clicking on a topic expands/collapses it and that no overlay is shown

When logged in user visits a talk page on desktop Minerva

  1. Login and visit https://en.wikipedia.beta.wmflabs.org/wiki/Talk:Dog?useskin=Minerva&useformat=desktop
  2. Ensure that there is NOT an "Read as wikipage" button, "There are no discussions on this page" message, or "Active discussions" message visible on the page. 3) Ensure the table of contents shows on wider screens
  3. Ensure that clicking on a topic expands/collapses it and that no overlay is shown

When logged in user visits a talk page with JS disabled

  1. Disable your browser's javascript and Login and visit https://en.wikipedia.beta.wmflabs.org/wiki/Talk:Dog
  2. Ensure that there is NOT a "Read as wikipage" button, "There are no discussions on this page" message, or "Active discussions" message visible on the page. There should still be an "Add discussion" button
  3. Ensure the table of contents shows on wider screens
  4. Ensure that there are edit icons next to the sections and clicking on one leads to an edit page for that section

When logged in, AMC user visits article

  1. Login with AMC mode on and visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Dog
  2. Ensure that there is a "Discussion" tab at top of page. Click on it and ensure you are taken to the talk page (no overlay appears).
  3. Repeat steps 3 - 10 from "When logged in, non-AMC user visits article"

QA Results

Related Objects

Event Timeline

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

Change 545420 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add onSaveComplete callback to TalkSectionOverlay

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

Change 546289 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Make add talk button show on talk page in AMC mode

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

Change 546685 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Mark the mobileview API as deprecated

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

Change 546747 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Make OverlayManager #add accept string literals

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

Change 546747 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Make OverlayManager #add accept string literals

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

@alexhollender and @nray please schedule some time together to design review this!

@nray and I just reviewed. Remaining items:

  • move Add discussion button to top of topics list (or create a second button there)
  • fix View as Wiki page button to bottom of viewport
  • add Active discussions header to top of topics list

Also noted the notion of "active discussions" which may or may not be relevant here

@Jdlrobson regarding ^^ do you know anything about the active discussions heading concept in the old talkBoard overlay and how feasible it is to add that into the new treatment?

active-discussions.png (1×3 px, 173 KB)

@Jdlrobson regarding ^^ do you know anything about the active discussions heading concept in the old talkBoard overlay and how feasible it is to add that into the new treatment?

active-discussions.png (1×3 px, 173 KB)

You can do this in SkinMinerva::prepareHeaderAndFooter by setting a value of postheadinghtml - note we already do something similar for user pages so there may be some light refactoring possible.

Status update: Alex has design review approved latest changes, this will go back in needs code review soon

Change 547768 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Add #remove method to OverlayManager

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

Change 547768 abandoned by Nray:
Add #remove method to OverlayManager

Reason:
Will handle this differently

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

Change 548543 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Correct the back button behaviour for talk overlay with unstaged changes

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

Change 548543 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Correct the back button behaviour for talk overlay with unstaged changes

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

Change 540441 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Render talk page as a tab instead of an overlay

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

A question: How flow talk pages will be rendered in this new version?

Change 545686 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Cleanup TalkSectionOverlay, TalkSectionAddOverlay, PageGateway

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

Change 545945 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove eventbus usage in talk.js

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

A question: How flow talk pages will be rendered in this new version?

@Masumrezarock100 Thanks for bringing this up. Flow talk pages shouldn't be affected by this change (they should work as before), although I did find a bug on the beta cluster (https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Flow_QA) that causes the add button/message to appear which I will need to fix before this hits production

Change 546228 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove obsolete 'mobile-frontend-talk-' messages

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

Change 548897 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Remove unneeded JS from OverlayManager _matchRoute

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

Change 548906 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Add support for non-ascii characters in OverlayManager#_matchRoute

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

nray updated the task description. (Show Details)

Change 548906 abandoned by Nray:
Add support for non-ascii characters in OverlayManager#_matchRoute

Reason:
thinking this should probably be handled upstream, not in overlaymanager

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

Change 548945 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/MinervaNeue@master] Add support for non-ascii characters in simplified talk page sections

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

I'm moving this to needs more work so i can finish the qa steps

@Edtadros this is now ready to be qa'd.

Unfortunately, through the process of writing some of these steps I know that some of them will fail, but fixes are on the way.

Change 548897 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Cleanup OverlayManager _matchRoute

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

Masumrezarock100 added a subscriber: Johan.

I think a User-notice to tech news recipients would be appropriate since this is a big design change for mobile talk pages. @Johan ping. :)

I think a User-notice to tech news recipients would be appropriate since this is a big design change for mobile talk pages. @Johan ping. :)

Since there's no train for the next couple of weeks, this won't go out until Thursday the 25th

@nray two things I've just noticed:

  1. If I open a discussion and then click the back button (without adding a reply), when I return to the main talk page it's scrolled all the way down.
  2. There are a lot of flashes when saving a reply and it's pretty disorienting. I don't remember it being like this before.


Some frames from the video:

Screen Shot 2019-11-14 at 1.18.08 PM.png (662×374 px, 83 KB)
Screen Shot 2019-11-14 at 1.13.04 PM.png (664×372 px, 82 KB)
Screen Shot 2019-11-14 at 1.13.14 PM.png (660×365 px, 81 KB)
Screen Shot 2019-11-14 at 1.13.31 PM.png (664×372 px, 73 KB)
Screen Shot 2019-11-14 at 1.13.42 PM.png (665×375 px, 81 KB)
Screen Shot 2019-11-14 at 1.20.47 PM.png (661×374 px, 95 KB)

If I open a discussion and then click the back button (without adding a reply), when I return to the main talk page it's scrolled all the way down.

From what I can see it scrolls to the thing that was clicked on. Going back takes me to where I was before in the document. Are you saying we want to scroll back to the top of the page in this circumstance? We can do that, we just need to make sure that's the appropriate behaviour here.

There are a lot of flashes when saving a reply and it's pretty disorienting. I don't remember it being like this before.

It's more noticeable now and relates to code we didn't get to refactor as part of the MobileFrontend architecture project but this this can be mitigated. I've talked to Nick in the channel about this a little.

If I open a discussion and then click the back button (without adding a reply), when I return to the main talk page it's scrolled all the way down.

From what I can see it scrolls to the thing that was clicked on. Going back takes me to where I was before in the document. Are you saying we want to scroll back to the top of the page in this circumstance? We can do that, we just need to make sure that's the appropriate behaviour here.

To clarify what I'm seeing:

without scrolling the page I click on Scoobydoothen I tap the <- buttonI land with the page scrolled
image.png (1×750 px, 106 KB)
image.png (1×750 px, 139 KB)
image.png (1×750 px, 113 KB)

In the above case I would expect the scroll position of the page to be the same as it was before I tapped into the discussion (which in this case is not scrolled down at all). Now I see that the page is scrolling me so that the conversation I'm coming from is right at the top of the screen. It would be better though if it just took me back to whatever scroll position was there when I tapped a discussion. If that's not possible I think landing with the page scrolled to the top would be preferable to what's currently happening.

Test Result

Status: ❌ FAIL
OS: macOS Mojave
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):

QA steps

✅ AC1: When anon user visits talk page directly

  1. Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Dog as an anon user

✅ 2) Ensure that an "Add discussion" button is NOT displayed
✅ 3) Ensure there is a "Read as wiki page" button at bottom of page
✅ 4) Ensure that clicking on a section opens up an overlay where you can see the topic title, comment, and any replies to that topic. You won't be able to add a reply since you are an anon,.
✅ 5) Click the "Read as wiki page" button. Ensure that it lets you see the page with a Table of Contents (on wide screens), but without the "Read as wikipage" button or the "Active discussions" header.
✅ 6) Click on a link in the TOC and ensure it leads you to the top of clicked section
✅ 7) Ensure that you can collapse/expand a section by clicking on it and that no overlay shows
✅ 8) Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:History/Talk:Main_Page?action=history and ensure that you don't see an "Add discussion button" or a "Read as wiki page" button

Screenshots for passing steps
230695-1.png (2×1 px, 179 KB)
230695-2.png (2×1 px, 172 KB)
230695-4.png (4×2 px, 679 KB)
230695-3.png (4×1 px, 550 KB)
230695-5.gif (294×638 px, 170 KB)
230695-6.png (2×1 px, 174 KB)

❌ AC2: When logged in, non-AMC user visits article

  1. Login and visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Dog

✅ 2) Ensure that there is a "Discussion" button at bottom of page. Click on it and ensure you are taken to the talk page (no overlay appears).
✅ 3) Ensure there is a "Read as wiki page" button at bottom of page, "Add discussion" button at top of page, and "Active discussions" heading with a list of topics below that
✅ 4) Ensure that clicking on a section opens up an overlay where you can see the topic title, comment, and any replies to that topic. Type a reply into the overlay and click the "Publish" button. Ensure that the page refreshes and the overlay appears again with the reply you typed visible (overlay should open again automatically). Exit overlay.
✅ 5) Click the "Add discussion" button and ensure an overlay appears that lets you add a discussion.
❌ 6) In the "Subject" field, type a subject with ascii and non-ascii characters e.g. ("😭\o/abc123!*"'();:@&=+$,/?#[]%F0%9F%98%AD存在%😭") should cover most of the edge cases out there. In the "What is on your mind?" field, type whatever you want. When done, click the "Publish" button. Ensure that the page refreshes and a toast appears saying your save was successful. Scroll to bottom of page and ensure your topic appears in the list. Click on the topic and ensure the overlay appears and shows what you wanted to publish.

The topic that was created with the ascii and non-ascii characters would not open.

230695-12.gif (647×298 px, 326 KB)

✅ 7) Click the "Read as wiki page" button. Ensure that it lets you see the page with a Table of Contents (on wide screens), but without the "Read as wikipage" button and without the "Active discussions" header.
❌ 8) Click on the topic you added in step 6 in the TOC and ensure it leads you to the top of the section.

230695-13.gif (294×638 px, 300 KB)

✅ 9) Ensure that you can collapse/expand a section by clicking on it and that no overlay shows
✅ 10) Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Main_Page?action=history and ensure that you don't see an "Add discussion button" or a "Read as wiki page" button

Screenshots for passing steps
230695-7.png (2×1 px, 156 KB)
230695-8.png (2×1 px, 171 KB)
230695-10.png (2×1 px, 274 KB)
230695-9.png (2×1 px, 246 KB)
230695-14.png (13×1 px, 1 MB)

❌ AC3: When logged in user visits a talk page with Flow enabled

  1. Login and visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Flow_QA

❌ 2) Ensure that there is NOT a Table of contents, "Add discussion" button, "Read as wikipage" button, "There are no discussions on this page" message, or "Active discussions" message visible on the page.

AMC OnAMC Off
230695-17.png (2×1 px, 290 KB)
230695-18.png (2×1 px, 291 KB)

✅ and ❌ 3) Ensure that clicking on a topic expands/collapses it and that no overlay is shown
This passes in regards to the functionality of expanding/collapsing. however, the down arrow is cut off and looks like a checkmark.

230695-19.gif (700×323 px, 951 KB)

❌ AC4: When logged in user visits a talk page on desktop Minerva

  1. Login and visit https://en.wikipedia.beta.wmflabs.org/wiki/Talk:Dog?useskin=Minerva&useformat=desktop

❌ 2) Ensure that there is NOT an "Read as wikipage" button, "There are no discussions on this page" message, or "Active discussions" message visible on the page.

230695-20.png (2×1 px, 266 KB)

❌ 3) Ensure the table of contents shows on wider screens
230695-21.png (2×2 px, 464 KB)

❌ 4) Ensure that clicking on a topic expands/collapses it and that no overlay is shown
230695-21.gif (863×398 px, 605 KB)

✅ AC5: When logged in user visits a talk page with JS disabled

  1. Disable your browser's javascript and Login and visit https://en.wikipedia.beta.wmflabs.org/wiki/Talk:Dog

✅ 2) Ensure that there is NOT a "Read as wikipage" button, "There are no discussions on this page" message, or "Active discussions" message visible on the page. There should still be an "Add discussion" button

230695-22.png (2×1 px, 276 KB)

✅ 3) Ensure the table of contents shows on wider screens
230695-23.png (2×1 px, 303 KB)

✅ 4) Ensure that there are edit icons next to the sections and clicking on one leads to an edit page for that section
230695-24.gif (480×360 px, 87 KB)

✅ AC6: When logged in, AMC user visits article

  1. Login with AMC mode on and visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Dog

✅ 2) Ensure that there is a "Discussion" tab at top of page. Click on it and ensure you are taken to the talk page (no overlay appears).
✅ 3) Repeat steps 3 - 10 from "When logged in, non-AMC user visits article"

230695-25.gif (690×318 px, 3 MB)

Change 551883 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Talk overlays should be synchronous

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

Will this go out in .8 as planned?

This will go out in december. No harm in announcing it early though.

Change 551883 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Talk overlays should be synchronous

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

In T230695#5664564, @alexhollender wrote:

@nray two things I've just noticed:

  1. If I open a discussion and then click the back button (without adding a reply), when I return to the main talk page it's scrolled all the way down.
  2. There are a lot of flashes when saving a reply and it's pretty disorienting. I don't remember it being like this before.


Some frames from the video:

Screen Shot 2019-11-14 at 1.18.08 PM.png (662×374 px, 83 KB)
Screen Shot 2019-11-14 at 1.13.04 PM.png (664×372 px, 82 KB)
Screen Shot 2019-11-14 at 1.13.14 PM.png (660×365 px, 81 KB)
Screen Shot 2019-11-14 at 1.13.31 PM.png (664×372 px, 73 KB)
Screen Shot 2019-11-14 at 1.13.42 PM.png (665×375 px, 81 KB)
Screen Shot 2019-11-14 at 1.20.47 PM.png (661×374 px, 95 KB)

@alexhollender Jon made a patch which should fix #1. It will also help minimize #2 but not totally resolve it as there are still three things happening:

  1. Page is refreshed to sync content of body with JS
  2. Browser jumps to top of section because the url contains a hash fragment (e.g. #foo) and the section html has an ID that is server side rendered which correspond to this fragment
  3. TalkSectionOverlay opens

I recommend opening a new ticket if this is still a problem as I don't think the fix will be trivial

I've reviewed failed AC steps:

Since we're tracking all of the failed AC, I think this is ready to move to signoff.

I recommend opening a new ticket if this is still a problem as I don't think the fix will be trivial

It looks better currently than it did when I took that video. Regarding further fixes does this task describe the work we'd need to do? T219388

Change 546685 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Mark the mobileview API as deprecated

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

In T230695#5725570, @alexhollender wrote:

I recommend opening a new ticket if this is still a problem as I don't think the fix will be trivial

It looks better currently than it did when I took that video. Regarding further fixes does this task describe the work we'd need to do? T219388

@alexhollender I think that ticket is more about lazy loading the overlay which we don't do anymore on the talk page. The remaining flash that happens when you reply to a talk topic is caused by the browser jumping to the section with an id that matches the hash fragment in the URL (#foo) before the overlay opens.