Skip to content
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

Unify menus with tooltips/popups #4607

Open
emilk opened this issue Jun 3, 2024 · 3 comments · May be fixed by #4669
Open

Unify menus with tooltips/popups #4607

emilk opened this issue Jun 3, 2024 · 3 comments · May be fixed by #4669
Labels
feature New feature or request refactor rerun Desired for Rerun.io

Comments

@emilk
Copy link
Owner

emilk commented Jun 3, 2024

egui menus (Ui::menu_button) and context menus (Response::context_menu) are implemented using the egui::menu module.

There are also tooltips (Response::on_hover_ui etc) and ComboBox popups, both implemented using egui::containers::popup.

It would be nice to unify these systems.

Things I would like to accomplish:

  • Being able to open a popup on:
    • hover (tooltip)
    • click (menu)
    • right-click or long-press (context menu)
  • Nest sub-menus everywhere (currently not possible in tooltips)
  • Call ui.close_menu to close a tooltip and popup (currently only works for menus)

At the same time it would be great to take a naming pass on all these things, and look up what they are commonly called in other ui libraries.

@Umatriz
Copy link
Contributor

Umatriz commented Jun 7, 2024

I would like to try to implement this. But I also want to be able to change the way egui::containers::popups are closed. Because for now if I add any kind of checkboxed into it or anything else that requires some kind of interaction the popup will immediately close as soon as I change something (mark a checkbox or press a button). I think it's better to let user to take control of it. Simitarly to the ui.menu_button and it's ui.close_menu.

@Umatriz
Copy link
Contributor

Umatriz commented Jun 7, 2024

I think I will make a PR and write all my thoughts about this isssue.

@emilk
Copy link
Owner Author

emilk commented Jun 7, 2024

Nice! I suspect there is a lot in menu.rs that can be simplified too

@Umatriz Umatriz linked a pull request Jun 18, 2024 that will close this issue
@emilk emilk added this to the Next Major Release milestone Jun 19, 2024
@emilk emilk added the rerun Desired for Rerun.io label Jun 19, 2024
Wumpf added a commit to rerun-io/rerun that referenced this issue Jun 20, 2024
### What

Previously, we had ui for adding component overrides, under an
experimental feature everywhere except the time series view + some
hardcoded ui for manipulating overrides (those that were
`EntityProperties` in 0.16).

This PR removes both the component override ui and the hardcoded
property ui in favor of a new overview + override ui showing all
currently used values by all active visualizers + ability to add/remove
visualizers.


https://github.com/rerun-io/rerun/assets/1220815/425a9406-9eb2-4319-846b-fe0d0eee1a42

---

Noteworthy "sideeffects":
* `ComponentUiCallback` now uses raw arrow arrays, allowing it to show
fallbacks
* kills lots'a old code :)
* `VisualizerQueryInfo::queried` keeps now archetype component ordering
instead of alphabetical (not in above video yet)
* lots of inconsistency in our data flow are now visible

---

Planned direct follow-ups:
* allow various reset / override menus 
* particularly important: remove override, add (single) override on
multi-value
* Would be great to do this with a menu button, will have to be a
context menu until emilk/egui#4607 is handled
* allow expanding to see the override/store/default/fallback stack
* original plan was to skip on this, but the method has all the data
layed out neatly now, so might as well!
* get the "Add" button integrated into the top (unless this is hard?)
*  show better docs: #6556
* Find a better name for the "blueprint section" below

---

Also needed but not planned short term:
* inspect multivalues
* we could do so for store multivalues already, but everything else we
can't yet select, so let's tackle this later
* ui polish

---

Part of:
* #4888

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6599?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6599?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6599)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
emilk pushed a commit that referenced this issue Jun 27, 2024
This PR adds `PopupCloseBehavior` to improve state of the
<#4607>

`PopupCloseBehavior` determines when popup will be closed.
- `CloseOnClick` popup will be closed if the click happens anywhere even
in the popup's body
- `CloseOnClickAway` popup will be closed if the click happens somewhere
else but in the popup's body.

It also adds a test in the demo app which contains several popups
examples.

---

My ideas about <#4607> is to make
every tooltip and popup a menu. So it will provide more control over
popups and tooltips (you will be able to close a popup by calling
something similar to the `ui.close_menu` if you need to). You won't need
to manually handle it's opening. And also will allow to have multiple
popups opened. That means you can have a popup inside a popup. And it
will also lead to the easier creation of the popups. (should we create a
tracking issue to track changes because to me it seems like a huge
amount of changes to be done?)

---

- Improvements on <#4607>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request refactor rerun Desired for Rerun.io
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants