Skip to content

Conversation

@viridia
Copy link
Contributor

@viridia viridia commented Oct 23, 2025

Objective

  • Splitting out the non-BSN parts of the work on popup menus.

Solution

  • Added Popover and Menu components.

Testing

  • Manual testing using example.

Showcase

popup

@viridia viridia marked this pull request as ready for review October 23, 2025 10:55
observe(on_menu_event),
children![(
Node {
width: Val::Px(200.0),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this is an example, where I expect to see idiomatic code, shouldn't the convenience functions (px, percent, px(n).all()) be used here and not Val directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced uses of Val.

@lomirus
Copy link
Contributor

lomirus commented Oct 24, 2025

The way this component is named Menu seems relatively uncommon, as in most front-end UI component libraries, it is typically referred to as Select:

@viridia
Copy link
Contributor Author

viridia commented Oct 24, 2025

The way this component is named Menu seems relatively uncommon, as in most front-end UI component libraries, it is typically referred to as Select:

"Select" is a different widget:

  • Menu displays a list of actions (open, close, quit, etc.)
  • Select displays a list of values that can be chosen

Both are in the class of "popups" and "dropdowns". You can easily build a select from a menu:

  • Create menuitems representing the individual choices
  • Modify the button label to reflect the current choice

@viridia viridia mentioned this pull request Oct 26, 2025
@alice-i-cecile alice-i-cecile added this to the 0.18 milestone Oct 27, 2025
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets M-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 27, 2025
@alice-i-cecile alice-i-cecile self-requested a review October 27, 2025 04:18
Copy link
Contributor

@isHavvy isHavvy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Approving only for my nit, not the whole thing)

Copy link
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only had a quick look, but I don't see any obvious problems with the implementation, and the menu in the example works nicely.

)
),
menu_button(asset_server),
Text::new("Press 'D' to toggle widget disabled states"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pressing "D" doesn't disable the menu, don't know if that's intended or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added code to disable the button, although I didn't add code to change the menu button color when disabled, but you can see that when disabled the menu does not pop up.

@github-actions
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21636

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@github-actions
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21636

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@github-actions
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21636

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@Zeophlite Zeophlite added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 29, 2025
Co-authored-by: Viktor Gustavsson <villor94@gmail.com>
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 29, 2025
Merged via the queue into bevyengine:main with commit 7d29266 Oct 29, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible M-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants