-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Unify menus with tooltips/popups #4607
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
Comments
I would like to try to implement this. But I also want to be able to change the way |
I think I will make a PR and write all my thoughts about this isssue. |
Nice! I suspect there is a lot in |
### 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`.
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>
This PR adds `PopupCloseBehavior` to improve state of the <emilk#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 <emilk#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 <emilk#4607>
Would greatly appreciate some condition flexibility for the helpers on Response, e.g. #3452 There are also a few additional spots in the code base that should probably be merged into whatever solution is chosen for this: e.g. https://github.com/emilk/egui/blob/master/crates/egui/src/widgets/color_picker.rs#L498 |
My plan is:
I also tried writing a test for popups that uses memory.everything_is_visible in the demo app but it was so chaotic that I don't think such a test has a lot of value. I'll rather write some more specific tests for the new popup. We should also pay attantion that the new popup/tooltip/menu plays together nicely with the modal. Maybe the open state handling could even be shared between popups and modals? Seems like it should be pretty similar |
This introduces new `Tooltip` and `Popup` structs that unify and extend the old popups and tooltips. `Popup` handles the positioning and optionally stores state on whether the popup is open (for click based popups like `ComboBox`, menus, context menus). `Tooltip` is based on `Popup` and handles state of whether the tooltip should be shown (which turns out to be quite complex to handles all the edge cases). Both `Popup` and `Tooltip` can easily be constructed from a `Response` and then customized via builder methods. This also introduces `PositionAlign`, for aligning something outside of a `Rect` (in contrast to `Align2` for aligning inside a `Rect`). But I don't like the name, any suggestions? Inspired by [mui's tooltip positioning](https://mui.com/material-ui/react-tooltip/#positioned-tooltips). * Part of #4607 * [x] I have followed the instructions in the PR template TODOs: - [x] Automatic tooltip positioning based on available space - [x] Review / fix / remove all code TODOs - [x] ~Update the helper fns on `Response` to be consistent in naming and parameters (Some use tooltip, some hover_ui, some take &self, some take self)~ actually, I think the naming and parameter make sense on second thought - [x] Make sure all old code is marked deprecated For discussion during review: - the following check in `show_tooltip_for` still necessary?: ```rust let is_touch_screen = ctx.input(|i| i.any_touches()); let allow_placing_below = !is_touch_screen; // There is a finger below. TODO: Needed? ```
Uh oh!
There was an error while loading. Please reload this page.
egui menus (
Ui::menu_button
) and context menus (Response::context_menu
) are implemented using theegui::menu
module.There are also tooltips (
Response::on_hover_ui
etc) andComboBox
popups, both implemented usingegui::containers::popup
.It would be nice to unify these systems.
Things I would like to accomplish:
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.
The text was updated successfully, but these errors were encountered: