Re-design PropertiesPanelTabButtons to hug contents instead of having equal widths#33765
Re-design PropertiesPanelTabButtons to hug contents instead of having equal widths#33765ajuncosa wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR advances the muse submodule to a new commit and updates the properties panel tab UI. PropertiesPanelTabBar now computes a readonly 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@muse`:
- Line 1: Update the PR description to clarify the submodule bump: explicitly
state that the change in muse/muse_framework (commit 65edd4a8... -> 6eadd9e0...)
only adds tooltips in StyledTabButton.qml and explain whether this tooltip
addition is part of the truncated-text/tab button fix for issue `#32604`; if it is
not related, split the tooltip change into its own PR referencing
StyledTabButton.qml and the specific commit so reviewers can review it
independently.
In
`@src/propertiespanel/qml/MuseScore/PropertiesPanel/common/PropertiesPanelTabButton.qml`:
- Around line 35-37: The overflow check in the calculation of
tabBarContentImplicitWidth omits the spacing between tabs, so update the reduce
logic that computes tabBarContentImplicitWidth (or compute a new total) to add
the total spacing: include tabBar.spacing * Math.max(0,
tabBar.contentChildren.length - 1) when comparing against tabBar.width; ensure
you reference tabBar.contentChildren.reduce (or the tabBarContentImplicitWidth
variable) and use tabBar.spacing and tabBar.contentChildren.length so truncation
correctly triggers when widths plus spacing exceed tabBar.width.
- Around line 35-50: PropertiesPanelTabButton.qml currently uses
tabBar.contentChildren and tabBar.count (variables tabBarContentImplicitWidth,
widthLimitForTruncation, huggedItems) which include hidden buttons; change the
math to only consider visible buttons by filtering tabBar.contentChildren with
button.visible to compute visibleChildren, visibleCount,
visibleContentImplicitWidth and visible huggedItems, then replace uses of
tabBar.count, total sums and spacing with these visible-aware values so
truncation and width distribution ignore hidden tabs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38e02acd-78cb-48e5-91a2-d2af07b2fa5b
📒 Files selected for processing (2)
musesrc/propertiespanel/qml/MuseScore/PropertiesPanel/common/PropertiesPanelTabButton.qml
0a3db75 to
ab0a3ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@muse`:
- Line 1: The muse submodule is configured to point to commit SHA
93ca12ae238a1be1c6c49f332891444af8c29c81 which does not exist in the
muse_framework repository. Either update the submodule configuration to
reference a valid commit SHA that exists in muse_framework (coordinate with the
muse_framework maintainers to confirm the correct commit), or remove the muse
submodule entirely from the .gitmodules file and the git index if it should not
be included in this repository. Ensure the chosen commit SHA has been pushed and
merged to the muse_framework repository before updating.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| fillWidth: true | ||
| fillWidth: false | ||
| font: ui.theme.bodyBoldFont | ||
| width: tabBar ? Math.min(implicitWidth, tabBar.truncatedItemWidth) : implicitWidth |
There was a problem hiding this comment.
If I'm not mistaken - referencing tabBar like this will give us "unqualified access" warnings since it isn't defined anywhere in this file. It'll work - but if someone comes in and changes the ID of tabBar further up then this will break and they'd be none the wiser.
Can we pass it in as a property instead? (perhaps a required property since there's no situation where these buttons should be used without a tab bar)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/propertiespanel/qml/MuseScore/PropertiesPanel/common/PropertiesPanelTabBar.qml`:
- Around line 48-55: The share calculation in the while loop can produce
negative values when root.width is smaller than the total inter-tab spacing,
which causes layout issues when passed to the Math.min() call for tab width.
After the while loop completes (after the closing brace), clamp the share
variable to ensure it is never negative by using Math.max(share, 0) before it is
used in any width calculations, so that tabs gracefully truncate to zero width
instead of receiving negative width values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b93d7b95-a62e-4455-a71b-dee2e94c23ee
📒 Files selected for processing (3)
musesrc/propertiespanel/qml/MuseScore/PropertiesPanel/common/PropertiesPanelTabBar.qmlsrc/propertiespanel/qml/MuseScore/PropertiesPanel/common/PropertiesPanelTabButton.qml
✅ Files skipped from review due to trivial changes (1)
- muse
🚧 Files skipped from review as they are similar to previous changes (1)
- src/propertiespanel/qml/MuseScore/PropertiesPanel/common/PropertiesPanelTabButton.qml
Resolves: #32604
Related to muse_framework: musescore/muse_framework#82 (Add tooltips to StyledTabButton)
Instead of evenly dividing the available tab bar width amongst the tab buttons, the buttons' width now fits their content (up to a maximum).
If the sum of the widths of all buttons surpasses the available tab bar width, then we identify some buttons to truncate by looking for buttons which surpass a threshold (this process may take multiple iterations).
Then, the widths of the buttons which are not to be truncated, simply fit their contents. The remaining tab bar width which is not occupied by "fitted" items is equally distributed amongst the "too wide" items, and their text is truncated accordingly.