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

For #22502 - Update "Jump back in" group's subtitle format #22506

Merged
merged 2 commits into from
Nov 22, 2021
Merged

For #22502 - Update "Jump back in" group's subtitle format #22506

merged 2 commits into from
Nov 22, 2021

Conversation

Mugurell
Copy link
Contributor

before after
image image

Pull Request checklist

  • 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. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

@Mugurell Mugurell requested review from a team as code owners November 19, 2021 12:27
@Mugurell Mugurell added the pr:needs-landing PRs that are ready to land [Will be merged by Mergify] label Nov 22, 2021
@mergify mergify bot merged commit 888bbc7 into mozilla-mobile:main Nov 22, 2021
@Mugurell Mugurell deleted the recentTabsText branch November 22, 2021 08:33
gabrielluong
gabrielluong reviewed Nov 23, 2021
View reviewed changes
app/src/main/res/values/strings.xml
The first parameter is the count for number of sites in the group. This number will always be more than one. -->
<string name="recent_tabs_search_term_count">Sites: %1$s</string>
%d is a placeholder for the number of sites in the group. This number will always be more than one. -->
<string name="recent_tabs_search_term_count_2">%d sites</string>
Copy link
Member

Choose a reason for hiding this comment

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

@Delphine @Mugurell I am wondering if this will break localization for Beta since we can't just remove a string like this iirc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, forgot about #20871.
Will add add the previous string back, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened #22572 to add back the previous string.

@Mugurell Mugurell mentioned this pull request Nov 24, 2021
Merged
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gabrielluong gabrielluong gabrielluong left review comments

@MozillaNoah MozillaNoah MozillaNoah approved these changes

Assignees
No one assigned
Labels
pr:needs-landing PRs that are ready to land [Will be merged by Mergify]
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Mugurell @gabrielluong @MozillaNoah