`ListItem`2.0 (part 1): introduce content-generic `ListItem` and `LabelContent` legacy back-port #6161

abey79 · 2024-04-29T18:14:38Z

What

This PR does the following:

Introduces the fundamental content-generic ListItem infrastructure (ListItem, trait ListItemContent, list_item_scope().
Introduces LabelContent, a ListItemContent implementation which implements the exact same features as the legacy ListItem.
Updates re_ui_example to demonstrate the use of the new list item, including a fairly extensive clean-up of the right panel code.

Part of Tracking issue: ListItem 2.0 #6075
Follow-up to ListItem 2.0 (part 0): re_ui_example refactor #6148
Fixes Remove ListItem clip-rect hack #5740

Limitation and todos

The handling of the X coordinate range for the background highlight needs (introduced here to part with the clip rect hack) needs splitting of to include all full-span widgets: Remove clip rect hack from every widget #6156.
The state management currently looks meaningless as state will only be used by the future PropertyContent. Funnily, all the state management currently does is what is to be split off as per above :)
Docstrings needs more work (in particular top-level overview)
ListItem + LabelContent should be deployed wherever we currently use ListItem 1.0, which should be then entirely removed.
And of course, we need a two-column PropertyContent…

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.

abey79 · 2024-05-01T07:10:17Z

crates/re_ui/src/list_item2/scope.rs

Note to reviewer: feedback welcome on the scope stuff, but it will be significantly overhauled in #6182. Also, the background range stuff will eventually be extracted out of list_item2 and generalised for all concerned widgets (#6156).

abey79 · 2024-05-01T07:24:00Z

crates/re_ui/src/list_item2/scope.rs

+    fn peek(ctx: &egui::Context) -> Option<&State> {
+        ctx.data(|reader| reader.get_temp(*STATE_STACK_ID))
+    }


This is outright wrong. Fixed in #6182

abey79 · 2024-05-01T14:21:34Z

crates/re_ui/src/list_item2/mod.rs

+        re_ui: &crate::ReUi,
+        ui: &mut egui::Ui,
+        context: &ContentContext<'_>,
+    ) -> Option<egui::Response>;


#6182 removes the return value.

abey79 · 2024-05-01T15:25:07Z

crates/re_ui/src/list_item2/scope.rs

+    - a global state stack that is read by actual list items
+     */
+
+    let id = id.into();


PR4 changes this into:

let id = ui.id().with(id.into());

This way, the id argument only needs to be unique to the "local" ui.

(i still manage to confuse myself with these sometime-nested id stuff 😓)

This way, the id argument only needs to be unique to the "local" ui.

Make sure that's documented in the docstring of the function

crates/re_ui/Cargo.toml

crates/re_ui/src/list_item2/list_item.rs

crates/re_ui/src/list_item2/mod.rs

crates/re_ui/src/list_item2/other_contents.rs

crates/re_ui/src/list_item2/scope.rs

emilk · 2024-05-02T07:58:35Z

crates/re_ui/src/list_item2/scope.rs

+    - a global state stack that is read by actual list items
+     */
+
+    let id = id.into();


This way, the id argument only needs to be unique to the "local" ui.

Make sure that's documented in the docstring of the function

emilk · 2024-05-02T07:59:52Z

crates/re_ui/src/list_item2/scope.rs

+    // read the state for this container, if any
+    let state: Option<State> = ui.data(|reader| reader.get_temp(id));
+    let mut state = state.unwrap_or_default();
+
+    // determine the background x range to use
+    state.background_x_range = if let Some(background_x_range) = background_x_range {
+        background_x_range
+    } else if let Some(parent_state) = StateStack::peek(ui.ctx()) {
+        parent_state.background_x_range
+    } else {
+        ui.clip_rect().x_range()
+    };
+
+    // push, run, pop
+    StateStack::push(ui.ctx(), state.clone());
+    let result = content(ui);
+    let state = StateStack::pop(ui.ctx());
+
+    // save the state for this container
+    if let Some(state) = state {
+        ui.data_mut(|writer| writer.insert_temp(id, state));
+    }


I'll just ignore all of this for this PR, since you say it's changing later anyways

abey79 · 2024-05-02T09:37:00Z

All review comments addressed in #6183

…property-like list items (#6174) ### What This PR introduced the `PropertyContent`, an implementation of `ListItemContent` for two-column list item, with a label and flexible "values". Currently only demonstrated in `re_ui_example`. What the "value" displays is delegated to a user-provided closure. However, `PropertyContent` provides helper for a few basic types: bool, text, color (both read-only and editable). - Part of #6075 - Follow-up to #6161 https://github.com/rerun-io/rerun/assets/49431240/bf94871a-63d5-46fa-94fe-a9adf720cdb8 ### Limitations and todo - Columns are fixed size at 50%. They will be made smart in [the next PR.](#6182) - More helpers are needed for various kinds of values. - There can be only 0 or 1 action button. This should be extended by using a `…` button with some kind of popup with all available actions in a future PR. - Right gutter space is reserved for the action button even if no list item in scope use them. The `list_item_scope` could track this and skip reserving that space if it's never used (e.g. component list in entity path selection panel): #6179 ### 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/6174?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/6174?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/6174) - [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`.

…ons when needed (#6183) ### What ☝🏻 This PR also slightly adjust the semantics of the `id` parameter of `list_item_scope()`. Now it just needs to be unique within the current UI. It also addresses review comments from #6161, #6174, and #6182. - Part of #6075 - Follow-up to #6182 - Fixes #6179 ![Export-1714577757563](https://github.com/rerun-io/rerun/assets/49431240/45cb8284-15a9-4100-8057-bd7fd994d008) <br/> Note: the upper group of (nested) items (which makes use of action buttons) has no impact on the lower group, because both sub-groups are in a distinct `list_item_scope`. ### 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/6183?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/6183?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/6183) - [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 Apr 29, 2024

abey79 changed the title ~~ListItem2.0 (part 1): generic ListItem setup and ListContent legacy back-port~~ ListItem2.0 (part 1): introduce content-generic ListItem and ListContent legacy back-port Apr 29, 2024

abey79 changed the title ~~ListItem2.0 (part 1): introduce content-generic ListItem and ListContent legacy back-port~~ ListItem2.0 (part 1): introduce content-generic ListItem and LabelContent legacy back-port Apr 30, 2024

abey79 mentioned this pull request Apr 30, 2024

ListItem 2.0 (part 2): introduce PropertyContent for two-column, property-like list items #6174

Merged

5 tasks

abey79 added 9 commits May 1, 2024 08:54

WIP basic structure

6eedc1b

WIP basic content

7be2372

WIP use in re_ui_example

5e61384

WIP update re_ui_ex with LI2 and made list_item_scope a function

65627c2

Working LabelContent and actually polished re_ui_example

932b5b0

lints

e6ccb72

rustdoc fixes

657caed

use workspace once_cell

ba7004c

fix cargo doc

413bf46

abey79 force-pushed the antoine/li1-basic branch from d00fcd8 to 413bf46 Compare May 1, 2024 06:55

abey79 commented May 1, 2024

View reviewed changes

emilk approved these changes May 2, 2024

View reviewed changes

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

Addressed review comments from #6161

f807b6f

abey79 mentioned this pull request May 2, 2024

ListItem 2.0 (part 4): only allocate space for property action buttons when needed #6183

Merged

5 tasks

abey79 merged commit f5cc5d7 into main May 2, 2024
33 checks passed

abey79 deleted the antoine/li1-basic branch May 2, 2024 13:38

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

Addressed review comments from #6161

bac703b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`ListItem`2.0 (part 1): introduce content-generic `ListItem` and `LabelContent` legacy back-port #6161

`ListItem`2.0 (part 1): introduce content-generic `ListItem` and `LabelContent` legacy back-port #6161

abey79 commented Apr 29, 2024 •

edited by github-actions bot

Loading

abey79 May 1, 2024 •

edited

Loading

abey79 May 1, 2024 •

edited

Loading

abey79 May 1, 2024 •

edited

Loading

abey79 May 1, 2024

abey79 May 1, 2024

emilk May 2, 2024

emilk May 2, 2024

emilk May 2, 2024

abey79 commented May 2, 2024 •

edited

Loading

ListItem2.0 (part 1): introduce content-generic ListItem and LabelContent legacy back-port #6161

ListItem2.0 (part 1): introduce content-generic ListItem and LabelContent legacy back-port #6161

Conversation

abey79 commented Apr 29, 2024 • edited by github-actions bot Loading

What

Limitation and todos

Checklist

abey79 May 1, 2024 • edited Loading

Choose a reason for hiding this comment

abey79 May 1, 2024 • edited Loading

Choose a reason for hiding this comment

abey79 May 1, 2024 • edited Loading

Choose a reason for hiding this comment

abey79 May 1, 2024

Choose a reason for hiding this comment

abey79 May 1, 2024

Choose a reason for hiding this comment

emilk May 2, 2024

Choose a reason for hiding this comment

emilk May 2, 2024

Choose a reason for hiding this comment

emilk May 2, 2024

Choose a reason for hiding this comment

abey79 commented May 2, 2024 • edited Loading

`ListItem`2.0 (part 1): introduce content-generic `ListItem` and `LabelContent` legacy back-port #6161

`ListItem`2.0 (part 1): introduce content-generic `ListItem` and `LabelContent` legacy back-port #6161

abey79 commented Apr 29, 2024 •

edited by github-actions bot

Loading

abey79 May 1, 2024 •

edited

Loading

abey79 May 1, 2024 •

edited

Loading

abey79 May 1, 2024 •

edited

Loading

abey79 commented May 2, 2024 •

edited

Loading