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

[CR132] Update the Tab groups icon (looks like four squares) in the bookmarks bar #42535

Closed
1 of 6 tasks
MadhaviSeelam opened this issue Nov 26, 2024 · 9 comments · Fixed by brave/brave-core#26925
Closed
1 of 6 tasks
Assignees
Labels
Chromium/upgrade major Major version bump. (ex: Chromium 88 to 89) OS/Desktop polish Nice to have — usually related to front-end/visual tasks priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/exclude
Milestone

Comments

@MadhaviSeelam
Copy link

MadhaviSeelam commented Nov 26, 2024

Description

Found while testing #42521.

Steps to reproduce

  1. Install 1.75.39 Chromium: 132.0.6834.15
  2. launch Brave
  3. verify the bookmarks bar

Actual result

Tab groups icon (looks like four squares) and is inherited from Chromium.

Image

Expected result

No tab groups icon similar to 1.73.x?

Image

Reproduces how often

Easily reproduced

Brave version (brave://version info)

Brave | 1.75.39 Chromium: 132.0.6834.15 (Official Build) nightly (64-bit)
-- | --
Revision | 4a2c1f978d65412aa0a7c6d2ad5c8f7a11b6a9af
OS | Windows 11 Version 23H2 (Build 22631.4460)

Channel information

  • release (stable)
  • beta
  • nightly

Reproducibility

  • with Brave Shields disabled
  • with Brave Rewards disabled
  • in the latest version of Chrome

Miscellaneous information

@rebron @emerick
cc: @brave/qa-team

@GeetaSarvadnya
Copy link

The issue is reproducible on macOS arm 64 - 1.75.39 Chromium: 132.0.6834.15

Image

@LaurenWags LaurenWags changed the title CR 132: Tab groups icon (looked like four squares) in the bookmarks bar is inherited from chromium CR132: Tab groups icon (looked like four squares) in the bookmarks bar is inherited from chromium Nov 27, 2024
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Dec 6, 2024
@aguscruiz
Copy link

aguscruiz commented Dec 6, 2024

Let's change the icon to our browser-group icon.
I'm not sure where else it's used (We talked about removing the option from the hamburger menu) but other occurences should be replaced too.

Image

Image

Image

@simonhong
Copy link
Member

Pushed brave/brave-core#26925 but it seems current master doesn't have browser-group icon yet.
Maybe we need nala bump for this?

@aguscruiz
Copy link

Yeah, we need a bump, I just created that icon today 😬

@simonhong
Copy link
Member

Yeah, we need a bump, I just created that icon today 😬

It seems latest nala bump doesn't include this - brave/brave-core#26995

@kjozwiak
Copy link
Member

kjozwiak commented Dec 16, 2024

@simonhong we'll probably want this one uplifted into 1.74.x as it's a C132 regression. Only if it's a simple uplift though. Probably shouldn't be bumping/upgrading Nala at this point as we always run into UI regressions throughout the browser when that happens. Should do that earlier in the cycle when needed. If this requires a Nala upgrade, we should wait and let it ride the trains via 1.75.x. If we can get a simple fix as per https://github.com/brave/brave-core/pull/26925/files, we can get this uplifted into 1.74.x.

@simonhong
Copy link
Member

@simonhong we'll probably want this one uplifted into 1.74.x as it's a C132 regression. Only if it's a simple uplift though. Probably shouldn't be bumping/upgrading Nala at this point as we always run into UI regressions throughout the browser when that happens. Should do that earlier in the cycle when needed. If this requires a Nala upgrade, we should wait and let it ride the trains via 1.75.x. If we can get a simple fix as per https://github.com/brave/brave-core/pull/26925/files, we can get this uplifted into 1.74.x.

@kjozwiak This change depends on latest Nala bump.

@kjozwiak
Copy link
Member

Gotcha, thanks @simonhong 👍 In that case, we can let this ride the trains. It's a really minor issue and uplifting nala introduces too much risk at this point re: fixing something minor as the above.

@rebron rebron changed the title CR132: Tab groups icon (looked like four squares) in the bookmarks bar is inherited from chromium [CR132] Update the Tab groups icon (looks like four squares) in the bookmarks bar Dec 18, 2024
@rebron rebron added the polish Nice to have — usually related to front-end/visual tasks label Dec 18, 2024
@MadhaviSeelam
Copy link
Author

Verification PASSED using

Brave | 1.75.157 Chromium: 132.0.6834.83 (Official Build) beta (64-bit)
-- | --
Revision | d8524431e89200eff892315294bd590b314f47d2
OS | Windows 11 Version 24H2 (Build 26100.2605)

Verified using STR from the description

Tab groups icon on the bookmarks bar seem clipped. Visually the icon doesn't seem to match to Figma design as it has more rounded corners than the icon shown - Failed

Filed a follow up - #43236

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chromium/upgrade major Major version bump. (ex: Chromium 88 to 89) OS/Desktop polish Nice to have — usually related to front-end/visual tasks priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants