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

Do not duplicate the space origin subtree in the blueprint tree UI #5371

Merged
merged 1 commit into from
Mar 2, 2024

Conversation

abey79
Copy link
Contributor

@abey79 abey79 commented Mar 1, 2024

What

Follow-up to:

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

image

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).

space_origin_link.mp4

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!

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

I think this is great :)
The only thing that bugs me is that the code for it adds an inconvenient bool to an already inconveniently large function. Not sure how to refactor but also not that important

@Wumpf
Copy link
Member

Wumpf commented Mar 1, 2024

removing from changelog since this wasn't duplicated in 0.14(.1) either

@Wumpf Wumpf added exclude from changelog PRs with this won't show up in CHANGELOG.md and removed include in changelog labels Mar 1, 2024
@abey79 abey79 merged commit 99b683a into main Mar 2, 2024
39 of 40 checks passed
@abey79 abey79 deleted the antoine/projection-origin-link branch March 2, 2024 07:42
abey79 added a commit that referenced this pull request Mar 2, 2024
…archical lists (#5362)

### 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:
- The collapsed state is now preserved when objects move in the
hierarchy (e.g. a space view is moved from a container to another).
Fixes #5208
- It is now possible to "remotely" change the collapsed state of a given
item, in a given scope.

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


https://github.com/rerun-io/rerun/assets/49431240/e0d6c47f-59f6-4be1-96d9-03d38d20a580

Blocked by:
- #5371

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
* [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/5362/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5362/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/5362/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/5362)
- [Docs
preview](https://rerun.io/preview/250f8e69ccbf28a29980a114ec8fa7ed535c05a8/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/250f8e69ccbf28a29980a114ec8fa7ed535c05a8/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛑 controversial exclude from changelog PRs with this won't show up in CHANGELOG.md ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants