Closed Bug 1409261 Opened 3 years ago Closed 3 years ago

PWA Badge and Onboarding - Add localization comments for pwa_add_to_launcher_badge

Categories

(Firefox for Android :: Web Apps (PWAs), enhancement, P1)

56 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 58
Tracking Status
firefox58 --- fixed

People

(Reporter: cnevinchen, Assigned: cnevinchen)

Details

(Whiteboard: [FNC][SPT58.3][INT][pwa-front-end] )

Attachments

(4 files)

follow up for bug 1393672
Summary: Add localization comments for pwa_add_to_launcher_badge → PWA Badge and Onboarding - Add localization comments for pwa_add_to_launcher_badge
Comment on attachment 8919143 [details]
Bug 1409261 - Update string and add translation note for pwa badge and onboarding.

https://reviewboard.mozilla.org/r/190054/#review195296

::: mobile/android/base/locales/en-US/android_strings.dtd:858
(Diff revision 1)
>  <!ENTITY private_tab_learn_more "Want to learn more?">
>  
>  <!ENTITY fullscreen_warning "Entered full screen">
>  
>  
>  <!ENTITY pwa_add_to_launcher_confirm "+ Add to Home Screen">

Please add a comment explaining why there's a + at teh beginning of the string.

::: mobile/android/base/locales/en-US/android_strings.dtd:862
(Diff revision 1)
>  
>  <!ENTITY pwa_add_to_launcher_confirm "+ Add to Home Screen">
> +
> +<!-- LOCALIZATION NOTE (pwa_add_to_launcher_badge): When there are more than three page actions, the second page action will become a drop down button.
> + After the user clicks on it, a drop down list will appears. The second page action and later ones will be on the list, but displayed as text
> + description instead of icons. The text below will be the description for pwa add to launcher page action.-->

Sorry, I really have a hard time imagining the layout based on this comment. Can you provide a screenshot? Also, what does it mean "description" in this context? Accessibility text, something else?

Final note: why are we calling out "PWA" (jargon) in a user facing string? I think that's incorrect, and it wasn't on the Invision mockup (unless I missed it).
Attachment #8919143 - Flags: review?(francesco.lodolo) → review-
Whiteboard: [pwa-front-end]
Should I reference the mockup / screen shot in the patch or in the bug here?
Thanks!
Flags: needinfo?(francesco.lodolo)
(In reply to Nevin Chen [:nechen] from comment #3)
> Should I reference the mockup / screen shot in the patch or in the bug here?

Non necessarily, I need it to understand where and how the string is used.
Flags: needinfo?(francesco.lodolo)
Priority: -- → P1
Nevin, can we prioritize this? It's blocking us from exposing other strings to localization tools, so I'd like to understand if we can get to a solution fast enough. The alternatives are to back out the patch, or expose the strings as they are, and they're both suboptimal.
Flags: needinfo?(cnevinchen)
I'll do it now. Sorry it's been a tough week.
Flags: needinfo?(cnevinchen)
Attached image onboarding.png
demostrate where "pwa_onboarding_sumo" and "pwa_continue_to_website" is
Attached image confirm.png
demonstrate where "pwa_add_to_launcher_confirm" and the plus sign is
Attached image MultiplePageActions.png
demonstrate where "pwa_add_to_launcher_badge" is
Comment on attachment 8919143 [details]
Bug 1409261 - Update string and add translation note for pwa badge and onboarding.

https://reviewboard.mozilla.org/r/190054/#review196110

Thanks, the screenshot helps a lot.

::: mobile/android/base/locales/en-US/android_strings.dtd:860
(Diff revision 2)
>  <!ENTITY fullscreen_warning "Entered full screen">
>  
> -
> +<!-- LOCALIZATION NOTE (pwa_add_to_launcher_confirm): The plus sign here is part of UI design -->
>  <!ENTITY pwa_add_to_launcher_confirm "+ Add to Home Screen">
> -<!ENTITY pwa_add_to_launcher_badge "Add PWA to Home Screen">
> +
> +<!-- LOCALIZATION NOTE (pwa_add_to_launcher_badge): When there are more than three page actions, the second page action will become a drop down button.

Suggested edit.

<!-- LOCALIZATION NOTE (pwa_add_to_launcher_badge2): Used as label in the page actions dropdown list, displayed when there are more than 3 actions available for a page. See also https://bug1409261.bmoattachments.org/attachment.cgi?id=8919897 -->

::: mobile/android/base/locales/en-US/android_strings.dtd:863
(Diff revision 2)
>  <!ENTITY pwa_add_to_launcher_confirm "+ Add to Home Screen">
> -<!ENTITY pwa_add_to_launcher_badge "Add PWA to Home Screen">
> +
> +<!-- LOCALIZATION NOTE (pwa_add_to_launcher_badge): When there are more than three page actions, the second page action will become a drop down button.
> + After the user clicks on it, a drop down list will appears. The second page action and later ones will be on the list, but displayed as text
> + description instead of icons. The text below will be the description for pwa add to launcher page action.-->
> +<!ENTITY pwa_add_to_launcher_badge "Add to Home Screen">

Unfortunately you need a new string ID here: I assumed pwa_add_to_launcher_badge2 in the comment above.

https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

To avoid this kind of issues in the future: was this string part of the UX mockups? If not, why wasn't it?
Attachment #8919143 - Flags: review?(francesco.lodolo) → review-
Comment on attachment 8919143 [details]
Bug 1409261 - Update string and add translation note for pwa badge and onboarding.

https://reviewboard.mozilla.org/r/190054/#review196138

::: commit-message-f474c:1
(Diff revision 3)
> +Bug 1409261 - Add translation note for pwa_add_to_launcher_badge. r=flod

Can you amend the commit message to reflect the current patch?
Attachment #8919143 - Flags: review?(francesco.lodolo) → review+
(In reply to Francesco Lodolo [:flod] from comment #13)
> Comment on attachment 8919143 [details]
> Bug 1409261 - Update string and add translation note for pwa badge and
> onboarding.
> 
> https://reviewboard.mozilla.org/r/190054/#review196138
> 
> ::: commit-message-f474c:1
> (Diff revision 3)
> > +Bug 1409261 - Add translation note for pwa_add_to_launcher_badge. r=flod
> 
> Can you amend the commit message to reflect the current patch?

Thanks. I triggered landing, hopefully it will land soon in m-c
Pushed by francesco.lodolo@mozillaitalia.org:
https://hg.mozilla.org/integration/autoland/rev/a3a899e97e27
Update string and add translation note for pwa badge and onboarding. r=flod
https://hg.mozilla.org/mozilla-central/rev/a3a899e97e27
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Whiteboard: [pwa-front-end] → [FNC][SPT58.3][INT][pwa-front-end]
You need to log in before you can comment on or make changes to this bug.