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

Add helpers to enable stable and controllable collapsed state in hierarchical lists #5362

Merged
merged 15 commits into from
Mar 2, 2024

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Feb 29, 2024

What

This PR introduces a set of helper objects to assist in producing predictable (and thus stable) egui::Id for various objects of our data model (container, space view, data result, etc.). Since the collapsed state is in general not shared across different UI area, this PR proposes a scoping mechanism (implemented with the type system), so the id for a given item is different depending on, e.g., its appearance in the blueprint tree or the streams tree.

This improves the collapsed state in hierarchical lists in two ways:

⚠️⚠️ Demo "Expend all/Collapse all" buttons are included in the container selection panel, to be reverted before merging.

stable_collapse_id.mp4

Blocked by:

To address in follow-up PRs:

  • fine-tune the DataResult representation such as to ensure sufficient specificity in case the DataResult appears multiple times in the space view blueprint sub-tree / cc @Wumpf
  • add "collapse all"/"expand all" context menu actions
  • improve "double-click focus" to uncollapse the focused items

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):
  • 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!

@abey79 abey79 added ui concerns graphical user interface include in changelog do-not-merge Do not merge this PR labels Feb 29, 2024
@abey79 abey79 changed the title Add helpers to ensure controllable and stable collapsed state in hierarchical lists Add helpers to enable stable and controllable collapsed state in hierarchical lists Feb 29, 2024
@emilk emilk self-requested a review March 1, 2024 08:43
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Nice! Looking forward to the "double-click to focus and expand everywhere" feature :)

@@ -694,6 +694,35 @@ fn container_top_level_properties(
},
);
}

// TODO: toy code, revert before merging
Copy link
Member

Choose a reason for hiding this comment

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

It would be kind of nice to have in the context menu 🤔

Especially if it was just one button, showing "expand all" unless it was already all expanded, in which case it would show "collapse all" instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! ...but in a follow up PR. This is blocked on sorting out the DataResult story.

crates/re_viewer_context/src/collapsed_id.rs Outdated Show resolved Hide resolved
@Wumpf
Copy link
Member

Wumpf commented Mar 1, 2024

double-click to focus and expand everywhere

you keep saying double-click but I thought we said it would be single-click? :). (Relatively easy to change nuance ofc, albeit the way 'focus' and 'selection' works maybe not entirely trivial)

@abey79
Copy link
Member Author

abey79 commented Mar 1, 2024

double-click to focus and expand everywhere

you keep saying double-click but I thought we said it would be single-click? :). (Relatively easy to change nuance ofc, albeit the way 'focus' and 'selection' works maybe not entirely trivial)

IIRC, single click would only (sister-)select without messing too much with the UI, and only double-click would "flash" the entire UI into showing the thing of interest.

And yeah, right this minute I'm interested in the infrastructure, so we are unencumbered when iterating on the actual UX.

abey79 added a commit that referenced this pull request Mar 2, 2024
…5371)

### What

Follow-up to:
- #5342

After #5342, it was possible for a data result subtree to be displayed
twice in the blueprint tree UI, for example:

<img width="778" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/13da8014-5abf-4206-bba8-d9d5a33d3919">

<br/>

This is a problem for storing collapsed state (#5362) and is generally a
weird UX.

This PR replaces the origin subtree by a link which, when clicked,
selects the actual origin subtree (which is always displayed above the
projections).


https://github.com/rerun-io/rerun/assets/49431240/6655a187-c786-4d30-8451-e473267e4526


### 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 newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5371/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5371/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5371/index.html?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/5371)
- [Docs
preview](https://rerun.io/preview/8f4012dfde1a2d023ed2493bb7009fd342989b53/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/8f4012dfde1a2d023ed2493bb7009fd342989b53/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@abey79 abey79 removed the do-not-merge Do not merge this PR label Mar 2, 2024
@abey79 abey79 merged commit f719d7e into main Mar 2, 2024
34 checks passed
@abey79 abey79 deleted the antoine/stable-collapse-id branch March 2, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve how collapsed state is saved for space views, containers, and DataResults in the Blueprint Tree UI
3 participants