`ListItem` 2.0 (part 5): deploy to the Visualizers and Overrides UIs #6184

abey79 · 2024-05-01T18:16:22Z

What

This PR deploys list_item2 to the visualizer and overrides UI currently used for timeseries spaces views.

Unsurprisingly, contact with the real world required some adjustments:

It's useful to be able to mix and match LabelContent and PropertyContent. The following changes were needed for that to look nice:
- Both contents have now a configurable min_desired_width.
- LabelContent now has a always_show_buttons option that perma-displays the buttons. This makes it easy to emulate PropertyContent's action button. This could be improved: List item: LabelContent should be able to have the exact same action button and behaviour as PropertyContent #6203
Now the left column width is computed based on the total width actually allocated by ListItem (as opposed to ui.max_rect() as seen from list_item_scope). This is more correct when the list is in a horizontal scroll area and fixes related visual glitches.

This PR also fixes the ShapeMarker editor UI, but required a workaround due to ui.max_rect() behaving weird when the ComboBox menu expends due to its contents.

Checklist

I have read and agree to Contributor Guide and the Code of Conduct
I've included a screenshot or gif (if applicable)
I have tested the web demo (if applicable):
- Using examples from latest main build: rerun.io/viewer
- Using full set of examples from nightly build: rerun.io/viewer
The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

…ployment to viZ/overrides UI

emilk · 2024-05-03T09:43:24Z

crates/re_data_ui/src/editors.rs

+            // workaround to force `ui.max_rect()` to reflect the content size
+            ui.allocate_space(egui::vec2(item_width, 0.0));


Suggested change

// workaround to force `ui.max_rect()` to reflect the content size

ui.allocate_space(egui::vec2(item_width, 0.0));

// workaround to force `ui.max_rect()` to reflect the content size

ui.set_width(item_width);

I see that the doscstring of ComboBox::width claims it sets the with of the menu… but it doesn't. We should fix that.

I think it's slightly more complicated than that, if I understand correctly. In my code, the width of the combobox itself can shrink a lot when squeezed: .width(ui.available_width().at_most(100.0)). The menu width correctly set to that size. The issue is when the menu content is larger than the provided width. In that case, the menu visually expends to accommodate its content, but doesn't reflect that in the closure's ui.available_width(), which remains stuck to combobox.width - inner_margins.

I'll try to have a repro and open an issue in egui.

Here: emilk/egui#4452

emilk · 2024-05-03T09:48:07Z

crates/re_data_ui/src/editors.rs

+                        let response = ctx
+                            .re_ui
+                            .list_item2()
+                            .selected(edit_marker == marker)
+                            .show_flat(
+                                ui,
+                                re_ui::list_item2::LabelContent::new(marker.to_string())
+                                    .min_desired_width(item_width)
+                                    .with_icon_fn(|_re_ui, ui, rect, visuals| {
+                                        paint_marker(ui, marker.into(), rect, visuals.text_color());
+                                    }),
+                            );


This exploded in line-count quite a bit… I wonder if there is anything we can do to improve this

I can't immediately think of one.

One area to think about is to have some DataUi like thing that would make our data objects somehow capable of producing impl ListItemContent. If MarkerShape had this behaviour, it could be passed directly (or as marker.as_content() or sth) to .show_flat. I think we do need some hindsight to be able to design such a system, though.

emilk · 2024-05-03T09:48:54Z

crates/re_data_ui/src/editors.rs

+            let background_x_range = ui
+                .spacing()
+                .menu_margin
+                .expand_rect(ui.max_rect())
+                .x_range();
+
+            re_ui::list_item2::list_item_scope(
+                ui,
+                "marker_shape",
+                Some(background_x_range),


we could make a re_ui::combo_box helper that also did this

Yes. I want all of our popup menu, context menu, etc. to be reimplemented and styled after list item:

Improve design implementation accuracy of dropdown menus #2734

I would certainly welcome your input in this area, especially around the menu logics that currently relies on custom egui::Ui function IIRC.

crates/re_ui/src/list_item2/scope.rs

crates/re_viewer/src/ui/override_ui.rs

crates/re_data_ui/src/editors.rs

abey79 · 2024-05-03T14:48:35Z

Review comment addressed in #6211

…ted module (#6211) ### What So far, the full span x coordinate rang needed for highlighting was maintained by `LayoutInfo` and `list_item_scope`. However, other widgets also need this mechanism. This PR splits of full-span management to a dedicated module, and deploys it both in `re_ui_example` and where `list_item2` is currently used. Other changes: - This PR also improves the sanity checks regarding both `list_item_scope` and `full_span_scope`. When missing, debug build will panic and release build will emit warnings. - Both scopes are now `ui.scope()`, so it's safe to modify `ui.style()` from the closure. - `list_item_scope` now sets `ui.spacing().item_spacing.y = 0`, since this is required anyway for `ListItem`s to look good. Follow-up PR will be needed to deploy `full_span` to all relevant widgets and their use in the viewer (#6156). - Part of #6075 - Part of #6156 - Follow-up to #6184 ### 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/6211?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/6211?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/6211) - [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`.

abey79 added ui concerns graphical user interface include in changelog labels May 1, 2024

abey79 force-pushed the antoine/li4-no-act-btn branch from 8bd03cc to cd0d0ab Compare May 2, 2024 14:18

Base automatically changed from antoine/li4-no-act-btn to main May 2, 2024 14:37

Update visualizer and overrides UI with list item2

f5c5132

abey79 force-pushed the antoine/li5-viz-ovrd-ui branch from 31f971d to f5c5132 Compare May 2, 2024 14:40

No plan survives contact with the real world: improvement based on de…

00388a3

…ployment to viZ/overrides UI

abey79 mentioned this pull request May 2, 2024

List item: LabelContent should be able to have the exact same action button and behaviour as PropertyContent #6203

Open

Don't use more width than available.

bf9f7d7

abey79 changed the title ~~[WIP] ListItem 2.0 (part 5): deploy to the Visualizers and Overrides UIs~~ ListItem 2.0 (part 5): deploy to the Visualizers and Overrides UIs May 2, 2024

abey79 added 2 commits May 2, 2024 20:32

Docstring improvements

80a681b

Merge branch 'refs/heads/main' into antoine/li5-viz-ovrd-ui

4b591e8

abey79 marked this pull request as ready for review May 2, 2024 18:33

abey79 marked this pull request as draft May 2, 2024 18:54

abey79 mentioned this pull request May 2, 2024

Fix visual glitch with MarkerShape override editor #6206

Closed

5 tasks

Better way to address min content width and fix for the markershape menu

9b16b01

abey79 marked this pull request as ready for review May 2, 2024 19:13

abey79 mentioned this pull request May 2, 2024

Tracking issue: ListItem 2.0 #6075

Open

11 tasks

Fix PropertyContent::value_text() to properly truncate

002f386

emilk approved these changes May 3, 2024

View reviewed changes

abey79 mentioned this pull request May 3, 2024

ListItem 2.0 (part 6): split full-span range management to a dedicated module #6211

Merged

5 tasks

abey79 added a commit that referenced this pull request May 3, 2024

Addressed review comments from #6184

49d2405

abey79 merged commit 0ac38a4 into main May 3, 2024
34 checks passed

abey79 deleted the antoine/li5-viz-ovrd-ui branch May 3, 2024 14:48

abey79 added a commit that referenced this pull request May 3, 2024

Addressed review comments from #6184

490a3c1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`ListItem` 2.0 (part 5): deploy to the Visualizers and Overrides UIs #6184

`ListItem` 2.0 (part 5): deploy to the Visualizers and Overrides UIs #6184

abey79 commented May 1, 2024 •

edited by github-actions bot

Loading

emilk May 3, 2024

emilk May 3, 2024

abey79 May 3, 2024

abey79 May 3, 2024

abey79 May 3, 2024

emilk May 3, 2024

abey79 May 3, 2024

emilk May 3, 2024

abey79 May 3, 2024 •

edited

Loading

abey79 commented May 3, 2024

		// workaround to force `ui.max_rect()` to reflect the content size
		ui.allocate_space(egui::vec2(item_width, 0.0));

ListItem 2.0 (part 5): deploy to the Visualizers and Overrides UIs #6184

ListItem 2.0 (part 5): deploy to the Visualizers and Overrides UIs #6184

Conversation

abey79 commented May 1, 2024 • edited by github-actions bot Loading

What

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abey79 May 3, 2024 • edited Loading

Choose a reason for hiding this comment

abey79 commented May 3, 2024

`ListItem` 2.0 (part 5): deploy to the Visualizers and Overrides UIs #6184

`ListItem` 2.0 (part 5): deploy to the Visualizers and Overrides UIs #6184

abey79 commented May 1, 2024 •

edited by github-actions bot

Loading

abey79 May 3, 2024 •

edited

Loading