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.

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

Uplift several sidebar fixes (uplift to 1.37.x) #12624

Merged
merged 5 commits into from
Mar 18, 2022

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Mar 16, 2022

Uplift of #12596 #12618 #12589 #12578 #12644

Resolves brave/brave-browser#21655
Resolves brave/brave-browser#21014
Resolves brave/brave-browser#21060
Resolves brave/brave-browser#21597
Resolves brave/brave-browser#21590
Resolves brave/brave-browser#21726

Pre-approval checklist:

  • You have tested your change on Nightly.
  • This contains text which needs to be translated.
    • There are more than 7 days before the release.
    • I've notified folks in #l10n on Slack that translations are needed.
  • The PR milestones match the branch they are landing to.

Pre-merge checklist:

  • You have checked CI and the builds, lint, and tests all pass or are not related to your PR.

Post-merge checklist:

  • The associated issue milestone is set to the smallest version that the changes is landed on.

fix brave/brave-browser#21060

Introduced new IDS_SETTINGS_SIDEBAR_SHOW_OPTION_TITLE.
Although it uses same string with IDS_SIDEBAR_SHOW_OPTION_TITLE,
made different string id to prevent future regression.
IDS_SIDEBAR_SHOW_OPTION_TITLE was shared by many other UI such as app menu
or context menu.
fix brave/brave-browser#21597

In some languages, header text is truncated due to long translated
message. To prevent this, header label length is not fixed.
@simonhong simonhong requested a review from a team March 16, 2022 05:44
@simonhong simonhong requested a review from a team as a code owner March 16, 2022 05:44
@simonhong simonhong self-assigned this Mar 16, 2022
@simonhong
Copy link
Member Author

simonhong commented Mar 16, 2022

This uplift PR will include #12578

fix brave/brave-browser#21590

So far, sidebar can't aligned with custom theme
because sidebar only knows brave's dark/light theme color.

With below changes, sidebar colors are aligned well with custom theme also.

Used toolbar color as a sidebar background color.
Used toolbar button's hover color for sidebar item/arrow button hover color.
Picked proper color between light/dark color for builtin item icon color based on toolbar color.
Used toolbar area separator color(separator between toolbar and web contents) for sidebar border.
@simonhong simonhong added this to the 1.37.x - Beta milestone Mar 16, 2022
fix brave/brave-browser#21726

On Stable, sidebar is not visible by default.
Otherwise, it's shown.
Copy link
Member

@kjozwiak kjozwiak left a comment

Choose a reason for hiding this comment

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

Uplift into 1.37.x approved 👍 QA has verified the PR on Nightly as per #12596 (comment), #12618 (comment), #12589 (comment), #12578 (comment) and #12644 (comment).

@kjozwiak kjozwiak merged commit 250eb3c into 1.37.x Mar 18, 2022
@kjozwiak kjozwiak deleted the uplift_sidebar_fixes_1_37 branch March 18, 2022 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants