Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is localized to UI rendering, aligns with existing menu_item layout behavior, and the updated call sites are straightforward migrations.
Pull request overview
User-visible UI impact: Context menu submenus now display icons (and match the visual sizing/alignment of regular menu items), including an “Add to playlist” icon and an iconized multi-artist “Go to artist” submenu.
This PR adds a reusable UI helper to render submenu rows with the same layout metrics as existing popup menu items, then updates a couple of context menus to use it so submenu entries can show an icon consistently.
Changes:
- Added
menu_submenuinsrc/ui/widgets.rsto render a popup menu row that opens a child submenu, including hover and open-state styling and a trailing submenu arrow. - Updated “Add to playlist” in both single-item and multi-selection context menus to use the new helper and display
Icon::ListPlus. - Updated the multi-artist “Go to artist” entry to use the new helper and display
Icon::User.
File summaries
| File | Description |
|---|---|
src/ui/widgets.rs |
Introduces menu_submenu and migrates relevant context menu submenus to use it with icon support. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
The submenu icons, row alignment and hover/focus styling are approved and integrated in 4294e55, with your authorship preserved. The local checks and hosted Linux, macOS, Windows and Nix checks pass. This is on main and has not been released yet. Thanks for the contribution and updated screenshots.
a1fdfe5 to
8380afa
Compare
|
Here are the before and after comparisons in both dark and light themes: Dark Theme
Light Theme
Verification & Accessibility
|
|
The visible changes are submenu icons, aligned row spacing and hover/focus styling. The supplied light/dark captures and keyboard regression address the earlier requests. Please rebase onto current main and add narrow-window captures in both themes so I can finish the visual review against the current menus. |
8380afa to
55490a9
Compare
|
Rebased onto current main. Narrow-Window (760px) Captures
All checks and tests pass ( |
Give Add to playlist and Go to artist submenu rows icons and consistent text, arrow, hover and focus layout. Preserve the existing child menus and actions. All nine contribution checks pass on the exact integrated tree; narrow and normal light/dark comparisons cover closed and open submenus.
Summary
Adds icon support to popup context submenus (
menu_submenu), aligning them with standard context menu items.Changes
menu_submenuhelper insrc/ui/widgets.rs, matchingmenu_itemlayout metrics (28px height, 16px icon slot, 26px text offset, BiDi layout, hover/open state) and appending the right arrow (egui::menu::SubMenuButton::RIGHT_ARROW).Icon::ListPlusin both single-item (item_menu) and multi-selection (picked_menu) context menus.item_menuto displayIcon::Userwhen multiple artists are present, consistent with the single-artist item.Verification
cargo run --features demo -- --demo --demo-page playlist:pl1.cargo fmt --all --check.cargo clippy --locked --all-targets -- -D warnings.cargo test --lib(310 passed).