Change 764328 had a related patch set uploaded (by Vadim Kovalenko; author: Vadim Kovalenko):

[mediawiki/services/mobileapps@master] Add optional talk page icon to the mobile-html endpoint

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

Hi @cooltey ! Could you clarify which event should trigger 'Talk page' icon visibility? There is 'Reading Focus mode' mentioned in T297630, how can I reproduce it to hook setTalkPageButton(isVisible)?

Added pcs handler for icon visibility + tests. Note: I suppose, this icon also requires Voice Over feature that can be implemented with the scope of the next stage.

Hi @vadim-kovalenko
The behavior is similar to this one:
https://github.com/wikimedia/mobileapps/blob/7c00242953b2a2f4554580e88bcac14b668d455a/pagelib/docs/pcs/pcs.md#seteditbuttonsiseditable-isprotected-onsuccess

When the reading focus mode is on, all "edit" buttons in the mobile-html are invisible, which in the app we will call the function above to hide all edit buttons.

In this ticket, we would like to have a new function called setTalkPageButton(isVisible), so that we can control the visibility of the talk page bubble and its number.

Please let me know if you need more information, thanks!

Hi @cooltey ! Thank you for the clarification, I've implemented this in the patch above.

Checking in - What is the status on this task? Is the patch still under review? Thanks!

Hi @MattCleinman ! Yes, it is still under review.

Change 764328 merged by jenkins-bot:

[mediawiki/services/mobileapps@master] Add optional talk page icon to the mobile-html endpoint

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

Looking at the ticket, I suppose we never explicitly said the default should be not visible, other than calling it “optional”.

Apologies for the late addition: The default should be that this is not visible. Nothing should change about the default display. But when setTalkPageButton(isVisible) is called w/ true, then this new display should show.

In T298814#7770434, @MattCleinman wrote:

But when setTalkPageButton(isVisible) is called w/ true, then this new display should show.

Ideally, showing the Talk bubble should also be settable during the setup phase, so that the bubble appears when the page is first rendered. If we need to call setTalkPageButton() after the page is already loaded, it might cause a layout change, which is not desirable.

Is this optional setup call used as a feature flag or showing the talk button is going to remain optional ?

In T298814#7788458, @Jgiannelos wrote:

Is this optional setup call used as a feature flag or showing the talk button is going to remain optional ?

The talk button should remain optional at all times. (The app has a reading-focus setting that hides all editing-related features, and the talk button is one of them.)

Change 773516 had a related patch set uploaded (by Vadim Kovalenko; author: Vadim Kovalenko):

[mediawiki/services/mobileapps@master] Add optional talk page icon to the mobile-html endpoint

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

Do we know when this will be complete and deployed? Thanks!

@MattCleinman Patch above is ready for review.
cc: @MSantos , @Jgiannelos

Change 773516 merged by jenkins-bot:

[mediawiki/services/mobileapps@master] Add optional talk page icon to the mobile-html endpoint

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

Confirmed that this is deployed and working, so unblocking the parent task. Thanks!

Hi @vadim-kovalenko

Just checked the implementation in the app, and I have noticed the bubble icon will make the description div move down a little bit. Would it be possible to avoid the layout jumps? Thanks!

cc @scblr

Demo video:
https://www.youtube.com/shorts/63oot342ivw

Hi @cooltey . Sure, I'll take a look.

Change 790349 had a related patch set uploaded (by Vadim Kovalenko; author: Vadim Kovalenko):

[mediawiki/services/mobileapps@master] Add optional talk page icon to the mobile-html endpoint

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

Thanks @cooltey for spotting this:

I have noticed the bubble icon will make the description div move down a little bit. Would it be possible to avoid the layout jumps? Thanks!

And @vadim-kovalenko for the great work so far!

👏👏👏

@vadim-kovalenko Thanks for your work on this. I want to make sure you saw the comment directly above this - this probably should move from the sign off column, to handle that tweak that is needed. Thank you!

Change 790349 merged by jenkins-bot:

[mediawiki/services/mobileapps@master] Fix glitch with layout while switching talk page icon Update HTML inside header

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

No giant rush, but do you know when this will be deployed?

In T298814#7986998, @MattCleinman wrote:

No giant rush, but do you know when this will be deployed?

Hey, @MattCleinman it should get into the next deployment window (tomorrow). If I get some time I'll do it before that :)

Hi @vadim-kovalenko

While wiring up the talk page bubble in the app, I found an issue when opening some articles.
https://en.wikipedia.org/wiki/Robert_Hoapili_Baker
https://en.wikipedia.org/wiki/Kal%C4%81kaua%27s_1881_world_tour

If we set talkPageButton: true, the article will stop loading and I can see the log has the following error messages, which maybe something went wrong when appending the child to the html.

2022-06-13 12:06:30.812 15616-18271/org.wikipedia.dev I/okhttp.OkHttpClient: <-- 200 https://en.wikipedia.org/api/rest_v1/page/mobile-html/Robert_Hoapili_Baker (215ms, unknown-length body)
2022-06-13 12:06:30.828 15616-15616/org.wikipedia.dev D/org.wikipedia.bridge.CommunicationBridge$CommunicatingChrome: onConsoleMessage():147: :0 - Error with Permissions-Policy header: Origin trial controlled feature not enabled: 'interest-cohort'.
2022-06-13 12:06:30.828 15616-15616/org.wikipedia.dev D/org.wikipedia.bridge.CommunicationBridge$CommunicatingChrome: onConsoleMessage():147: :0 - Error with Permissions-Policy header: Unrecognized origin: 'intake-analytics.wikimedia.org'.
2022-06-13 12:06:30.829 15616-15616/org.wikipedia.dev I/chatty: uid=10523(org.wikipedia.dev) identical 3 lines
2022-06-13 12:06:30.829 15616-15616/org.wikipedia.dev D/org.wikipedia.bridge.CommunicationBridge$CommunicatingChrome: onConsoleMessage():147: :0 - Error with Permissions-Policy header: Unrecognized origin: 'intake-analytics.wikimedia.org'.
2022-06-13 12:06:30.869 15616-15616/org.wikipedia.dev D/org.wikipedia.bridge.CommunicationBridge$CommunicatingChrome: onConsoleMessage():147: https://meta.wikimedia.org/api/rest_v1/data/javascript/mobile/pcs:1 - Uncaught TypeError: Cannot read properties of undefined (reading 'appendChild')

Could you please help to check the issue? Thanks!

Change 805336 had a related patch set uploaded (by Vadim Kovalenko; author: Vadim Kovalenko):

[mediawiki/services/mobileapps@master] Add additional check of the DOM elem when append talk page icon in the header

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

Hi @cooltey ! I suspect that happens when you try to set the talk page icon before all other HTML has been rendered. In the current implementation, this button is configured inside the onBodyEnd() function here and this is expected because we need the header to render first to append the icon to it. I have a few questions that might help me to reduce the scope of the problem:

1. Do all articles throw an error when you try to set talkPageButton: true or the only these two mentioned in your comment?
2. Do you try to set it before HTML has been loaded? In that case, it won't work.
3. Maybe you could explain how you wire the button in the app itself so I can also be able to reproduce this issue?

In T298814#8002191, @vadim-kovalenko wrote:

Hi @cooltey ! I suspect that happens when you try to set the talk page icon before all other HTML has been rendered. In the current implementation, this button is configured inside the onBodyEnd() function here and this is expected because we need the header to render first to append the icon to it. I have a few questions that might help me to reduce the scope of the problem:

1. Do all articles throw an error when you try to set talkPageButton: true or the only these two mentioned in your comment?

No. I noticed the same articles will not always have the error. It could happen on any articles.

2. Do you try to set it before HTML has been loaded? In that case, it won't work.

Some articles can show the talk bubble and the entire article without any issue, but some articles cannot load the entire article correctly.

3. Maybe you could explain how you wire the button in the app itself so I can also be able to reproduce this issue?

We add the talkPageButton: true in the PCS setup function: https://github.com/wikimedia/mobileapps/blob/7c00242953b2a2f4554580e88bcac14b668d455a/pagelib/docs/pcs/pcs.md#setup
https://github.com/wikimedia/apps-android-wikipedia/blob/talk-page-bubble-design/app/src/main/java/org/wikipedia/bridge/JavaScriptActionHandler.kt#L90-L110

and we will add the JavaScript interface when it initializes the communication bridge class between the app and the mobile-html: https://github.com/wikimedia/apps-android-wikipedia/blob/5cc95ec6bc326fe13e2f5d2a5e12112033a795b7/app/src/main/java/org/wikipedia/bridge/CommunicationBridge.kt#L50, which will pass the setup() function to the bridge class.

and then we will call the function to actually load the mobile-html.

We haven't changed this behavior for quite a long time, and I am not sure if that causes the issue.

One possible thing might be the cache issue since it happened on some random articles that have fewer views, and maybe we should wait until all pages are cached if it was the issue.

Just found another related issue:

When loading any user page, such as User: CFeng_(WMF), the entire mobile-html will not be loaded successfully.
https://en.wikipedia.org/api/rest_v1/page/mobile-html/User:CFeng_(WMF)

Not sure if this also relates to the issue that I addressed in the previous comment.

@cooltey apparently, this is somewhat related to parsoid output, see: https://en.wikipedia.org/api/rest_v1/page/html/User:CFeng_(WMF)

I'll investigate.

The page doesn't exist on enwiki, only on meta,
https://meta.wikimedia.org/api/rest_v1/page/html/User:CFeng_(WMF)

See https://www.mediawiki.org/wiki/Extension:GlobalUserPage

T153801 seems related

In T298814#8011788, @Arlolra wrote:

The page doesn't exist on enwiki, only on meta,
https://meta.wikimedia.org/api/rest_v1/page/html/User:CFeng_(WMF)

See https://www.mediawiki.org/wiki/Extension:GlobalUserPage

Thanks @Arlolra and @MSantos
I am sorry that I gave a wrong example to you. The issue that I mentioned will happen on any existing user page.

Hi @vadim-kovalenko and cc @MattCleinman and @MSantos

As we discussed in the meeting, here is the link to the document showing the way of testing the local mobile-html in the Android app.
https://docs.google.com/document/d/1zEE9PRCBSvgVm1eDxMQnhz-yBU3vfbFv7LT2kuWzd5o/edit

@MSantos , @Jgiannelos pls take a look at this patch:
https://gerrit.wikimedia.org/r/c/mediawiki/services/mobileapps/+/805336

Steps to reproduce: run the app, go to http://localhost:8888/en.wikipedia.org/v1/page/mobile-html/User:Cooltey, in dev tools run pcs.c1.Page.setTalkPageButton(true). My concern is that solution is overcomplicated - I need to add extra DOM nodes for adding/removing the icon in a different ways for pages with and without header. The trickiest part is to preserve the default edit icon position which has float:right style. To simplify it, I probably need to do medium/major refactoring of the very first section rendering + refactor existing tests. What do you think of the current solution? Can we temporarily keep it?

cc: @cooltey

Hi @vadim-kovalenko

Would it be easier if we update the design so that you don't need to take care of the DOM issue? Or do you have any suggestion that can have the talk bubble icon have at least 48px*48px size and let it float to the right?

Hi @cooltey . Please, take a look at the screenshot:

Hi @vadim-kovalenko

Sounds good to me if that can solve the issue. Please make sure that the first icon should be able to trigger the context menu in the app.

This means it still needs to contain the data-action="edit_section"
https://github.com/wikimedia/mobileapps/blob/bb5b1fd4430ccd495264ae2150d088f70cde6f7b/pagelib/src/transform/EditTransform.js#L32 data-action="edit_section"

@vadim-kovalenko - Hold off on this one for now. Sorry, I just found out that the design on this may change. We'll let you know when we're confident how we'd like to move forward with this.

This can be deprioritized, we will monitor what the Web team learns by making talk page entry points more prominent since this has been technically challenging.

Change 805336 abandoned by Vadim Kovalenko:

[mediawiki/services/mobileapps@master] Implement talk page icon for pages without the header

Reason:

Abandoned because of design issues.

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