Toolbar - tweak responsive behaviour with new options#276450
Conversation
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the toolbar's responsive behavior by replacing the boolean responsive option with a more granular responsiveBehavior object that includes both an enabled flag and an optional minItems setting. This allows consumers to specify a minimum number of items that should always remain visible in the toolbar.
- Renamed
responsiveoption toresponsiveBehaviorwith structured configuration - Added
minItemscapability to enforce a minimum number of visible toolbar items - Refactored
setToolbarMaxWidthtoupdateActionswith improved readability - Implemented minimum width calculation based on item count
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/vs/base/browser/ui/toolbar/toolbar.ts | Core implementation of the responsive behavior refactoring with new responsiveBehavior API and minimum items logic |
| src/vs/workbench/contrib/scm/browser/scmRepositoryRenderer.ts | Updates SCM repository toolbar to use the new responsiveBehavior API with minItems: 2 |
| // takes precedence over the action label. | ||
| return this.actionBar.length() * ACTION_MIN_WIDTH; | ||
| } | ||
| const actionBarWidth = () => this.actionBar.length() * ACTION_MIN_WIDTH; |
There was a problem hiding this comment.
The actionBarWidth function is called multiple times in conditional checks and loops within the same method (lines 292, 296, 309, 338). Consider computing the value once and reusing it, or storing it in a variable that's updated when this.actionBar.length() changes to avoid redundant calculations.
| this.container.style.minWidth = `${itemCount * ACTION_MIN_WIDTH}px`; | ||
| this.element.style.minWidth = `${itemCount * ACTION_MIN_WIDTH}px`; |
There was a problem hiding this comment.
The minimum width calculation itemCount * ACTION_MIN_WIDTH is duplicated on lines 258 and 259. Consider extracting this to a local variable to reduce duplication and improve maintainability.
| this.container.style.minWidth = `${itemCount * ACTION_MIN_WIDTH}px`; | |
| this.element.style.minWidth = `${itemCount * ACTION_MIN_WIDTH}px`; | |
| const minWidthPx = `${itemCount * ACTION_MIN_WIDTH}px`; | |
| this.container.style.minWidth = minWidthPx; | |
| this.element.style.minWidth = minWidthPx; |
No description provided.