-
-
Notifications
You must be signed in to change notification settings - Fork 196
helium/ui: reduce side panel minimum width #674
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could this be merged into helium/ui/side-panel.patch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 0b58449
| // The default and minimum acceptable side panel content width. | ||
| - static constexpr int kSidePanelDefaultContentWidth = 360; | ||
| + static constexpr int kSidePanelDefaultContentWidth = 260; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change will break basically all side panels, because they're not optimized to be scaled this small. all of them will look bad by default.
"default" and "minimum" should be separated, and side panel size should snap to "default" while being resized back and forth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Hi @wukko and @dumbmoron, thank you for taking time to review this PR during the holidays (Happy new year!), and pointing out the oversight by simply changing the default constant. I've updated the patch to separate minimum from default, so that side panels use the original default width but are allowed to scale smaller if the user wish to do so. Snapping back to default width is also added. Thanks again for your time and suggestions. |
3b04ffb to
82766b8
Compare
For your pull request to not get closed without review, please confirm that:
If such issue did not exist before, I opened one. (I've opened [FR]: Reduce Side Panel Minimum Width #673)
otherwise I have marked my PR as draft.
organization if I lied by checking any of these checkboxes.
Tested on (check one or more):
closes #673
I've add a patch that decrease the minimum side panel width from 360px to 260px, so that it will not take up too many horizontal spaces on a laptop screen. The default width is kept the same as 360px and I've also added snapping to default width on resizing.
This is done by adding a constant
kSidePanelMinimumContentWidththat defines the fixed minimum width in chrome/browser/ui/views/side_panel/side_panel_entry.h to260(px). Then use this constant inGetMinimumSize()function in side_panel.cc so it only affects the minimum width while the default width stays the same.Snapping is done by adding a function
GetDefaultSize()that useskSidePanelMinimumContentWidthto calculate width, and update theOnResizefunction to see if theproposed_widthis close enough to the default width, if so then snap to that width.This change should allow user to define a smaller extension side panel width if they wish, while keeping the default to not break existing extensions.
Thanks again for this wonderful browser!
Screenshots
Default Chromium 360px
Updated 260px
Snapping to default width
Screen.Recording.2026-01-01.at.5.06.36.PM.mov