Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed Tabs menu position for some languages #27982

Merged
merged 2 commits into from
Mar 19, 2025
Merged

Fixed Tabs menu position for some languages #27982

merged 2 commits into from
Mar 19, 2025
+111 −0

Conversation

AlexeyBarabash
Copy link
Contributor

@AlexeyBarabash AlexeyBarabash commented Mar 5, 2025 •
edited by kjozwiak
Loading

Resolves brave/brave-browser#44274
Resolves brave/brave-browser#44278

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  1. Switch browser interface language to FR
  2. Ensure there is a correct preferences order in the main app settings

@AlexeyBarabash AlexeyBarabash added the CI/skip Do not run CI builds (except noplatform) label Mar 5, 2025
@AlexeyBarabash AlexeyBarabash self-assigned this Mar 5, 2025
@AlexeyBarabash AlexeyBarabash removed the CI/skip Do not run CI builds (except noplatform) label Mar 18, 2025
@AlexeyBarabash AlexeyBarabash force-pushed the fix_44274_fr_onglets_order branch 2 times, most recently from 909bb43 to 6162db8 Compare March 18, 2025 14:41
@AlexeyBarabash AlexeyBarabash mentioned this pull request Mar 18, 2025
Closed
6 tasks
@AlexeyBarabash AlexeyBarabash marked this pull request as ready for review March 18, 2025 15:51
deeppandya
deeppandya approved these changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@deeppandya deeppandya left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for adding test for sorted prefs

SergeyZhukovsky
SergeyZhukovsky approved these changes Mar 18, 2025
View reviewed changes
Copy link
Member

@SergeyZhukovsky SergeyZhukovsky left a comment

Choose a reason for hiding this comment

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

++

samartnik
samartnik approved these changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@samartnik samartnik left a comment

Choose a reason for hiding this comment

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

lgtm

@github-actions GitHub Actions
Copy link
Contributor

Chromium major version is behind target branch (134.0.6998.95 vs 135.0.7049.17). Please rebase.

@github-actions github-actions bot added the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Mar 19, 2025
@AlexeyBarabash AlexeyBarabash force-pushed the fix_44274_fr_onglets_order branch from 6162db8 to 0aa7ea0 Compare March 19, 2025 12:43
@github-actions github-actions bot removed the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Mar 19, 2025
@AlexeyBarabash
Copy link
Contributor Author

Chromium major version is behind target branch (134.0.6998.95 vs 135.0.7049.17). Please rebase.

done, Mr. Robot

@AlexeyBarabash AlexeyBarabash merged commit 498a827 into master Mar 19, 2025
18 checks passed
@AlexeyBarabash AlexeyBarabash deleted the fix_44274_fr_onglets_order branch March 19, 2025 18:52
@github-actions github-actions bot added this to the 1.78.x - Nightly milestone Mar 19, 2025
@brave-builds
Copy link
Collaborator

Released in v1.78.54

brave-builds added a commit that referenced this pull request Mar 21, 2025
@brave-builds
Uplift of #27982 (squashed) to beta
6e4d64b
brave-builds added a commit that referenced this pull request Mar 21, 2025
@brave-builds
Uplift of #27982 (squashed) to release
6365dac
This was referenced Mar 21, 2025
Merged
Merged
@hffvld
Copy link
Collaborator

hffvld commented Mar 21, 2025

Verified on Galaxy Tab S8 and Pixel 7 using version(s):

Device/OS: 
- Galaxy Tab S8 / gts8wifixx-user 14 UP1A.231005.007 release-keys
- Pixel 7 / panther_beta-user 16 BP22.250103.008 release-keys
Brave build: 1.78.57
Chromium: 135.0.7049.26 (Official Build) canary (64-bit)
Fresh install

STEPS:

  1. Switch device to English, French or Macedonian language, for example
  2. Launch Brave
  3. Hamburger menu > Settings
  4. Scroll down and find Tabs > Verify that the Tabs preference is shown right below the Display title and above the Media preference while the system is set to any language

ACTUAL RESULTS:

  • Verified that the Tabs preference is shown right below the Display title and above the Media preference while the system is set to any language.

Galaxy Tab S8

English German French Macedonian
1 2 3 4

Pixel 7

English Russian French Macedonian
1 2 3 4
Upgraded profile

STEPS:

  1. Switch device to English, French or Macedonian language, for example
  2. Install and launch Brave 1.78.51
  3. Hamburger menu > Settings
  4. Scroll down and find Tabs > Observe that the Tabs preference is shown above the Media preference in English, above the Display title in French and Macedonian
  5. Close and upgrade Brave to 1.78.57
  6. Launch Brave > Hamburger menu > Settings
  7. Scroll down and find Tabs > > Verify that the Tabs preference is shown right below the Display title and above the Media preference while the system is set to any language

ACTUAL RESULTS:

  • Verified that the Tabs preference is shown right below the Display title and above the Media preference while the system is set to any language.

Galaxy Tab S8

English German French Macedonian
1 2 3 4
1 2 3 4

Pixel 7

English Russian French Macedonian
1 2 3 4
1 2 3 4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emerick emerick emerick approved these changes

@deeppandya deeppandya deeppandya approved these changes

@SergeyZhukovsky SergeyZhukovsky SergeyZhukovsky approved these changes

@samartnik samartnik samartnik approved these changes

Assignees

@AlexeyBarabash AlexeyBarabash

Labels
None yet
Projects
None yet
Milestone
1.78.x - Nightly
7 participants
@AlexeyBarabash @brave-builds @hffvld @emerick @deeppandya @SergeyZhukovsky @samartnik