Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

No issue: Update long-press shortcut strings #5284

Merged
merged 2 commits into from
Sep 26, 2019

Conversation

jyeontaek
Copy link
Contributor

@jyeontaek jyeontaek commented Sep 12, 2019

We're updating the strings by request of @apbitner

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

@jyeontaek jyeontaek requested review from a team as code owners September 12, 2019 23:42
@jyeontaek jyeontaek added this to the v2.1 milestone Sep 12, 2019
@codecov-io
Copy link

codecov-io commented Sep 13, 2019

Codecov Report

Merging #5284 into master will decrease coverage by 0.51%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5284      +/-   ##
============================================
- Coverage     13.24%   12.73%   -0.52%     
+ Complexity      289      277      -12     
============================================
  Files           253      251       -2     
  Lines         10404    10326      -78     
  Branches       1514     1502      -12     
============================================
- Hits           1378     1315      -63     
+ Misses         8932     8922      -10     
+ Partials         94       89       -5
Impacted Files Coverage Δ Complexity Δ
...n/java/org/mozilla/fenix/IntentReceiverActivity.kt 0% <0%> (-33.34%) 0% <0%> (-6%)
...nix/home/sessioncontrol/SessionControlComponent.kt 0% <0%> (-1.36%) 0% <0%> (ø)
.../src/main/java/org/mozilla/fenix/utils/Settings.kt 70.54% <0%> (-0.6%) 15% <0%> (ø)
...va/org/mozilla/fenix/components/metrics/Metrics.kt 24.74% <0%> (-0.39%) 0% <0%> (ø)
...org/mozilla/fenix/crashes/CrashReporterFragment.kt 0% <0%> (ø) 0% <0%> (ø) ⬇️
...pp/src/main/java/org/mozilla/fenix/FeatureFlags.kt 90% <0%> (ø) 2% <0%> (ø) ⬇️
...a/org/mozilla/fenix/components/IntentProcessors.kt 0% <0%> (ø) 0% <0%> (ø) ⬇️
.../org/mozilla/fenix/customtabs/CustomTabsService.kt 0% <0%> (ø) 0% <0%> (ø) ⬇️
...ug/java/org/mozilla/fenix/DebugFenixApplication.kt 0% <0%> (ø) 0% <0%> (ø) ⬇️
...a/org/mozilla/fenix/exceptions/ExceptionDomains.kt 0% <0%> (ø) 0% <0%> (ø) ⬇️
... and 58 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb77015...bc9f10e. Read the comment docs.

liuche
liuche previously requested changes Sep 13, 2019
@@ -42,9 +42,9 @@

<!-- Home screen icons - Long press shortcuts -->
<!-- Shortcut action to open new tab -->
<string name="home_screen_shortcut_open_new_tab">Open new tab</string>
<string name="home_screen_shortcut_open_new_tab">New tab</string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How long has this string been landed for? To be safe, you probably need to update the resource name of this string, because it will already have been translated by L10N. Changing strings requires a new resource name so it gets picked up by Pontoon, and localizers see it as a new string to be localized.

@Delphine just to check, that's still necessary with the a10n localization Fenix is using, right?

home_screen_shortcut_open_new_tab2 or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This string got landed last week Tuesday.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay got an answer in #fenix-team, yes we still do need to update the resource ids.

@jyeontaek jyeontaek removed this from the v2.1 milestone Sep 13, 2019
@sblatz sblatz changed the title No issue: Update long-press shortcut strings RE RUN TC No issue: Update long-press shortcut strings Sep 20, 2019
@sblatz sblatz changed the title RE RUN TC No issue: Update long-press shortcut strings No issue: Update long-press shortcut strings Sep 20, 2019
@ekager
Copy link
Contributor

ekager commented Sep 23, 2019

Remember we shouldn't be landing new strings at this point in the sprint. We should punt landing this to after we cut next release?

@sblatz sblatz merged commit 59e2c12 into mozilla-mobile:master Sep 26, 2019
sblatz pushed a commit to sblatz/fenix that referenced this pull request Sep 30, 2019
* No issue: Update long-press shortcut strings

* Updates identifiers
sblatz pushed a commit to sblatz/fenix that referenced this pull request Sep 30, 2019
* No issue: Update long-press shortcut strings

* Updates identifiers
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants