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

Selection history should be global #5736

Closed
abey79 opened this issue Apr 2, 2024 · 2 comments · Fixed by #5739
Closed

Selection history should be global #5736

abey79 opened this issue Apr 2, 2024 · 2 comments · Fixed by #5739
Assignees
Labels
ui concerns graphical user interface

Comments

@abey79
Copy link
Member

abey79 commented Apr 2, 2024

Currently, the selection history is stored on a per-recording basis. However, with recent changes, this make no longer sense:

  • Blueprints are also recordings, and having a selection state there is meaningless.
  • Recording (and soon even AppIDs Make App ID selectable #5672) can now be selected. Since that also changes the current recording, the selection history becomes meaningless in that case.

To repro the behaviour:

  • open multiple recordings
  • select first recording
  • select a bunch of things from that recording
  • check selection history (right-click on back icon) -> notice there is stuff there
  • select another recording
  • check selection history -> such empty...
@abey79 abey79 added the ui concerns graphical user interface label Apr 2, 2024
@nikolausWest
Copy link
Member

How does that play with a future ability to set selection state from code?

@abey79
Copy link
Member Author

abey79 commented Apr 2, 2024

How does that play with a future ability to set selection state from code?

If anything this might make it actually easier since there is now a single place where the selection states lives.

Unrelated: this is technically a functional regression. Before, we could have a different selection state for each recording (e.g "/world/robot" is selected in recording A and "/world/camera" is selected in recording B). However, in practice that's no longer true since recording have been made selectable: switching to a new recording now implies selecting said recording, in turn clearing the selection. This issue would make it harder to go back to the previous behaviour.

jleibs added a commit that referenced this issue Apr 2, 2024
### What

The selection history is now global, so the back/forward button work as
expected when selecting various recordings.

Note (quoting my comment in the original issue):
> This is technically a functional regression. Before, we could have a
different selection state for each recording (e.g "/world/robot" is
selected in recording A and "/world/camera" is selected in recording B).
However, in practice that's no longer true since recording have been
made selectable: switching to a new recording now implies selecting said
recording, in turn clearing the selection. This issue would make it
harder to go back to the previous behaviour.

- Fixes #5736 


https://github.com/rerun-io/rerun/assets/49431240/d376605c-5306-4377-bcfa-acd9479c54d7


### 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/5739/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5739/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/5739/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/5739)
- [Docs
preview](https://rerun.io/preview/3c9cbb2c945b6af1ddba49b0838de6227f8a4a2c/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/3c9cbb2c945b6af1ddba49b0838de6227f8a4a2c/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

---------

Co-authored-by: Jeremy Leibs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui concerns graphical user interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants