Looks good to me on 2.7.50282-alpha-2019-07-10

Not quite sure why this hasn’t been moved to Design signoff first as there are obviously issues with it @Sharvaniharan ?

Reopen in the Nanaimo workboard @Charlotte ?

In T224728#5337371, @schoenbaechler wrote:

Not quite sure why this hasn’t been moved to Design signoff first as there are obviously issues with it @Sharvaniharan ?

I moved it to QA sign off directly since it would otherwise hold up testing while you were away @schoenbaechler
Should have probably made a note of it and let you know. sorry about that. Have put in a fix. Will move it to re-design review after it is merged.

Thanks @Sharvaniharan, looking forward to see the updated version!

On another note @Charlotte, I realized that in the card’s description, I wrote:

(...) the CTA’s below images are only displayed when there’s no structured image caption available on Commons, and when the user has Suggested Edits unlocked.

I think it doesn’t have the desired effect when we’d only show it to users that have unlocked Suggested edits. I suggest to show the CTA to all users in order to increase the feature’s visibility. Only of course, when there’s no structured image caption available on Commons – that condition will stay the same.

Yes, @schoenbaechler - I think you are right about the visibility. We will have to monitor the revert rate rather closely, but I think it is worth the slight risk to see what kind of engagement we get.

I agree @Charlotte, the effects of it are going to be interesting. Let’s try it out and honestly, we can remove the CTA again if we notice high revert rates or if it’s generally perceived in a negative way.

@Sharvaniharan could you enable the CTA for all users?

Thanks!

ok @schoenbaechler @Charlotte . will do that. will be visible to all logged in users

Thanks @Sharvaniharan - yes, we definitely do want logged-in folks only.

It works as intended, great job @Sharvaniharan. I know we discussed parts of this on Slack but adding it here for documentation purposes:

01) CTA should sit lower in the image and be bottom aligned. The canvas height of the element is 48dp and it should be vertically centered within it. Here’s a comparison (Implementation vs Design):

02) The element should span full width, the 48dp bottom area should be tappable and lead users to the caption edit page.

03) In regards to the snackbar at the bottom. Two things: The language output in parenthesis is different from other places. It outputs an abbreviation - in this case... it should be “Deutsch” instead of “de”. Plus, the “View” link is missing that should take users to the gallery view. To quote the task’s description: Tapping view takes users to the gallery view with the updated, structured caption.

@schoenbaechler ready for sign off!

Thanks @Sharvaniharan, the previous issues have been fixed. Looks wonderful! Unfortunately I noticed other issues 🙈:

01) After adding a caption in the secondary language, “View” (in the snackbar) takes users to the gallery view in the primary language:

Steps to reproduce:

1. Add a caption in the secondary language of the app via “Add image caption (%language)” CTA on the article page
2. Publish it and tap on “View” in the snackbar

Expected behavior:
“View” takes users to gallery view in the secondary language (in my case: German) and should be color-group-3

Actual behavior:
“View takes users to gallery view in the primary language (in my case: English) and uses a green color...

02) Also, after manually switching to the second language (in my case: German) on the article page to check out the German caption I just added, I see this:

Expected behavior:
The caption in the secondary language (in my case: German) is displayed in the gallery view and the “Add image caption (English)” CTA is hidden as there’s already an English (structured) caption available.

Actual behavior:
The caption in the primary language (in my case English) is displayed in the gallery view and the “Add image caption (English)” CTA is displayed, though there’s an English (structured) caption available.

CC’ing @Charlotte as I think these conceptual issues should be fixed before releasing.

Yes, @schoenbaechler these should be fixed.

1. @schoenbaechler Thank you ..Will fix this.

The green color in the snackbar for action button is a standard across the app in all the places where we use this kind of snackbar. We set it all in just one place. Can I change it so it is color-group-3 for all, or should we leave it green to maintain uniformity, or should this be different?

Also, generally when the user is reading the article in secondary language and opens the gallery, but there is no structured caption present in that language, it should fallback to the primary language, or to the unstructured caption in the secondary language.

@schoenbaechler @Charlotte This is a blocking bug that happens because of an unexpected response from the api returning structured edits. We will have to notify RI team and will be blocked until it is resolved. Great catch @schoenbaechler

cc @Dbrant @cooltey

@Charlotte here is the ticket I have created for RI : https://phabricator.wikimedia.org/T228774

I wan not sure of the priority on this. Could you please update to indicate the urgency.

@Sharvaniharan - It seems like from what @JoeWalsh was saying on T228774 that we can change the call we're making rather than asking for change work from RI? Have I understood correctly?

@Charlotte @Sharvaniharan my comments on T228774 were before I realized this was blocking release. Long term the app should switch to media-list and the other calls mentioned in T228774, if this is an immediate need blocking release it might be easier to fix the media endpoint. I'll investigate and report back here.

@Charlotte @Sharvaniharan patch for review is linked on the other bug. If that goes through it'd be a better short term solution, but switching to media-list is still the long term solution

Thanks for the heads-up @Sharvaniharan and for working on it so diligently (also @JoeWalsh).

@Sharvaniharan

The green color in the snackbar for action button is a standard across the app in all the places where we use this kind of snackbar. We set it all in just one place. Can I change it so it is color-group-3 for all, or should we leave it green to maintain uniformity, or should this be different?

We should switch to a color from our palette. I just checked if color_group_3 works from an accessibility perspective. Unfortunately it doesn’t, so we need to use accent75 for the VIEW label in all themes. I created a new color group for this: color_group_52. Thanks for adapting.

In T224728#5359257, @JoeWalsh wrote:

@Charlotte @Sharvaniharan patch for review is linked on the other bug. If that goes through it'd be a better short term solution, but switching to media-list is still the long term solution

Thank you for this @JoeWalsh

@Charlotte Gauthier @schoenbaechler, we will re-release tomorrow once the design sign-off is done on this ticket, which will also give us some time to keep testing ..

@Sharvaniharan @cooltey, not quite sure if these are related with the caching issues you mentioned on Slack.

01) It mainly looks/works well, but after successfully adding a structured caption to a file in English (Rinku premium outlets01s3200.jpg) on the Izumisano article page, and tapping the edit button in the gallery again, I see this:

The unstructured input field should be hidden after adding a structured caption

02) In addition, after successfully adding a caption in the secondary language of the app (German in my case) and tapping the edit button in the gallery view, I see this:

Strangely, it outputs the initial unstructured caption in English (the field says “Deutsch” though) and it should actually not be displayed at all, since a structured caption has been added previously

Tested in 2.7.50285-alpha-2019-07-26.

Thanks for your work! 👩‍🚒👨‍🚒

Hi @schoenbaechler

As we discussed on Slack, we decided to hide the "image description (%lang)" view after the user added a caption to the image, and the change also applies to the "Suggested edits" flow to make it consistently.

@Sharvaniharan Please move it to Design signoff once it gets merged, thank you.

Thank you for making this change @cooltey . Appreciate it. Will move it .

Moved back to design sign-off @schoenbaechler

Hey @cooltey @Sharvaniharan, the aforementioned issues are fixed, thanks for that. Unfortunately I observed the issues below. Sorry for being so pedantic about this but since exposure is so high for this (~ 200k logged in users per day will see it) it needs to work perfectly:

01)

Precondition:
One language is set in the app.

Steps to reproduce:
01) Find an article with no image caption (structured)
02) Tap the image, go to gallery view
03) Tap the edit button
04) Add an image caption (structured)

Expected behavior:
Users get back to the gallery view (where they left)

Actual behavior:
Users get back to the article page

02)

Precondition:
Multiple languages are set in the app.

Steps to reproduce:
01) Find an article with no image caption (structured) in the second language of the app
02) Tap the image, go to gallery view
03) Tap the “Add image caption (%secondarylanguage)” CTA
04) Add an image caption (structured)
05) Tap back arrow from the gallery view

Expected behavior:
“Add image caption (%secondarylanguage)” CTA on the article’s lead image is not displayed anymore.

Actual behavior:
“Add image caption (%secondarylanguage)” CTA on the article’s lead image is visible.

Note for QA:

01)

Precondition:
One language is set in the app.

Steps to reproduce:
01) Find an article with no image caption (structured)
02) Tap the image, go to gallery view
03) Tap the edit button
04) Add an image caption (structured)

Expected behavior:
Users get back to the gallery view (where they left)

Actual behavior:
Users get back to the article page

This issue happens when the app has only one app language set up.

@Sharvaniharan
Please move the ticket to the Design signoff after the PR merged.

Merged... @schoenbaechler ready for sign-off

@Sharvaniharan @cooltey

Issues mentioned in T224728#5374101 are fixed, but noticed new ones:

01)

Precondition:
Multiple languages are set in the app.

Steps to reproduce:
01) Find an article with no image caption in the %primarylanguage (structured)
02) Tap the image, go to gallery view

Expected behavior:
“Add image caption (%primarylanguage)” is consistently shown when there’s no structured caption in the %primarylanguage.

Actual behavior:
“Add image caption (%primarylanguage)” is inconsistently shown when there’s no structured caption in the %primarylanguage.

02)

Precondition:
Multiple languages are set in the app (in my case English + German).

Steps to reproduce:
01) Open this article: https://en.wikipedia.org/wiki/Badminton_at_the_1992_Summer_Olympics?wprov=sfla1
02a) Tap the “Add image caption (%primarylanguage)” CTA from the article page
02b) Tap on lead image, go to gallery view
02c) Tap on lead image, go to gallery view, tap the “Edit” button from the gallery view

Expected behavior:
Consistent output of the edit screen and gallery view, where the existing image description (unstructured) is shown. Gallery view should also output a CTA which says “Add image caption %primarylanguage” when there’s no image caption (structured) available. That way, it’s consistent with the CTA on the article page’s lead image.

Actual behavior:
Inconsistent output, see

From article head image CTA	Gallery view	From gallery edit button

Hi @schoenbaechler

The core issue of 02) is:

https://en.wikipedia.org/api/rest_v1/page/media/Badminton_at_the_1992_Summer_Olympics

If looks closely, you will notice that the Titles is File:Olympic_rings.svg, but the file_page is https://commons.wikimedia.org/wiki/File:Olympic_rings_without_rims.svg.

File:Olympic_rings.svg will be redirected to the File:Olympic_rings_without_rims.svg, and we should make a double-check the filename from both Titles and file_page URL.

If the 02) gets fixed, then I think 01) will be fixed, too.

I've already made a PR of fixing the issue, @Sharvaniharan please review it and merge it if it looks good to you.

Also, please take a look at issue 01) to see if my PR actually fixes it. (since I cannot reproduce it anymore).

I have merged it... and I did not completely understand 1) - is it that for any random image which doesn't have a structured caption in primary language, only sometimes the article CTA is shown and other times it is not? - I could not reproduce that.

@schoenbaechler
ready for review..

Hey @Sharvaniharan, 02) looks good to me!

In regards to 01), here’s an example. Both articles below lack a structured image caption.

A: Check out this article in the app (inconsistent):
https://en.wikipedia.org/wiki/Armand_de_Vignerot_du_Plessis

B: Now check out this article in the app (consistent):
https://en.wikipedia.org/wiki/Badminton_at_the_1992_Summer_Olympics

We need to always show the CTA in the gallery when there’s no structured caption available. Also, what users see on an article page needs to correlate with the gallery view. It’s about making the experience consistent. Long story short: A should also display a CTA in gallery view.

CC: @cooltey

@schoenbaechler I found that the root cause for this is that, on Gallery cta, we hide the explicit cta, since we have the edit button, only when there is neither structured nor unstructured caption available. However, we show it when there is an unstructured caption available, but no structured. However, on article cta we show in either case, which i feel is what should be the case. I have created a PR for correcting this logic, but please let me know if we need to do otherwise.
Thank you!

@schoenbaechler PR got merged and ready for design signoff now.

Looks good @Sharvaniharan & @cooltey, moving it to QA signoff!

https://www.youtube.com/watch?v=3GwjfUFyY6M

🎉🎉🎉

Yay!!

Looks good to me on 2.7.50286-r-2019-07-30