Skip to content

Adding context menu example #19245

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

Merged
merged 14 commits into from
May 26, 2025

Conversation

extrawurst
Copy link
Contributor

@extrawurst extrawurst commented May 16, 2025

Objective

Provides usage example of a context menu

Testing

  • Tested on MacOS
  • Tested on wasm using bevy_cli

Showcase

Screenflick.Movie.120.mp4

@extrawurst extrawurst changed the title context menu example draft Adding context menu example May 16, 2025
Copy link
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

@IQuick143 IQuick143 added C-Docs An addition or correction to our documentation C-Examples An addition or correction to our examples A-UI Graphical user interfaces, styles, layouts, and widgets S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 17, 2025
@extrawurst extrawurst marked this pull request as ready for review May 17, 2025 08:42
Copy link
Member

@Vrixyz Vrixyz left a comment

Choose a reason for hiding this comment

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

  • I'm approving if the CI goes green ; this PR is conflicting with New cooldown example #19234 and the other PRs adding the new usage folder.

@extrawurst
Copy link
Contributor Author

@Vrixyz added a bunch more docs and cleanups. happy to hear what you think

@extrawurst extrawurst requested a review from Vrixyz May 18, 2025 09:54
@extrawurst extrawurst removed the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label May 18, 2025
@Vrixyz
Copy link
Member

Vrixyz commented May 18, 2025

I like this example, other than a small nit on docs which may not be blocking, I added a comment on there which may be important to make sure we understand what this example does: https://github.com/bevyengine/bevy/pull/19245/files/eda7098b2b4e9a281253705b458cab263420fc57#r2094492441

@Vrixyz Vrixyz added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label May 18, 2025
@BenjaminBrienen BenjaminBrienen added the D-Straightforward Simple bug fixes and API improvements, docs, test and examples label May 18, 2025
Copy link
Member

@NiklasEi NiklasEi left a comment

Choose a reason for hiding this comment

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

Some small comments, but I like this example!

Copy link
Contributor

@BenjaminBrienen BenjaminBrienen left a comment

Choose a reason for hiding this comment

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

I think it would be good to have the context menu only appear when Mouse2 (or secondary pointer input button) is pressed, as that is more common.

@extrawurst
Copy link
Contributor Author

@BenjaminBrienen I would love to not do that for two reasons:

  1. it will make the example not work on mobile browsers (would like this to be working out of the box on the web as well when folks check it out on phones)
  2. it adds complexity (not much of course but nothing that cant be found in other examples)

@BenjaminBrienen
Copy link
Contributor

I don't think floating context menus are the appropriate way of getting input from touchscreen users, but I think your points are very valid.

@NiklasEi NiklasEi 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 May 18, 2025
@alice-i-cecile
Copy link
Member

This is probably going to get merge conflicts, but I'll try merging and see if anything breaks <3

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch May 26, 2025
@BenjaminBrienen BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels May 26, 2025
@BenjaminBrienen
Copy link
Contributor

Has merge conflicts 😋

@extrawurst
Copy link
Contributor Author

@BenjaminBrienen Ahh the new BorderColor's - should be fixed now

@BenjaminBrienen BenjaminBrienen 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-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 26, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 26, 2025
Merged via the queue into bevyengine:main with commit 0a11091 May 26, 2025
36 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-Docs An addition or correction to our documentation C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples 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.

7 participants