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

Implement 3-state visibility control #5464

Closed
abey79 opened this issue Mar 11, 2024 · 1 comment · Fixed by #5508
Closed

Implement 3-state visibility control #5464

abey79 opened this issue Mar 11, 2024 · 1 comment · Fixed by #5508
Assignees
Labels
ui concerns graphical user interface
Milestone

Comments

@abey79
Copy link
Member

abey79 commented Mar 11, 2024

Removing groups from the blueprint tree (#5326) resulted in a regression on visibility control. We decided to address it with a tri-state, inherited, visibility property (huddle minutes), whereby a given data result visibility is either unset (in which case it is inherited from parent), overridden to visible, or overridden to hidden. This means that the "eye" button needs to be tristate as well.

The initial proposal is to rotate over the 3 states with each click, and use 3 distinct icon, such as these:
image

@jleibs's proposal below is a better shot, at least as initial implementation.

For the selection panel, the simplest change would be to change the check box to a 3-item combobox.

@abey79 abey79 added the ui concerns graphical user interface label Mar 11, 2024
@abey79 abey79 self-assigned this Mar 11, 2024
@abey79 abey79 added this to the 0.15 milestone Mar 11, 2024
@jleibs
Copy link
Member

jleibs commented Mar 11, 2024

There is a tri-state, but I still disagree that it needs a tri-state button + icon in the blueprint tree.

There is nearly zero value in the sequence:

  • Hide parent
  • Hide child of hidden parent

The intuitive UI pattern is to have toggle always toggle the state.
The challenge, when toggling is that in some cases there are two different ways to toggle the state.

Specifically, this happens when a parent has a value, and a child has the opposite value. When toggling the child we can either set the value explicitly, or we can clear it letting it take on the value of the parent.

My proposal is to always prefer clearing unless clearing the value would not achieve the intended toggle. This means in most cases the toggle is a toggle between 2 states: cleared and opposite-of-parent. Occasional sequencing may result in a parent and child having the same value, in which case toggle should still do the correct thing on first toggle, after which it can return to cleared on the second toggle.

In the selection panel, I think there is value to showing a proper and explicit tri-state, but this is a much different design context.

@abey79 abey79 changed the title Implement 3-state visibility control in the blueprint tree Implement 3-state visibility control Mar 11, 2024
Wumpf added a commit that referenced this issue Mar 15, 2024
…5508)

### What

* Fixes #5464
* using the design we agreed upon that doesn't use tristate buttons in
the blueprint panel
* Next step on #5463
* Fixes #5396
* Fixes #3194 

The title describes the main effect this has on what Rerun can do
compared to the previous release. But the real star of the show is that
visible is now a component, not part of `EntityProperties`.

@ reviewer: Please take your time and explore the behavior both on
blueprint panel and selection panel, there's a surprising amount of
nuance in here.
For instance there's extra logic for allowing you to go back to a clean
slate with ui interactions - for instance hiding & unhiding an item that
doesn't receive any parent overrides will remove the visibility override
completely as the default value for visibility is 'true'.


You can now hide/show under a shown/hidden parent tree:


https://github.com/rerun-io/rerun/assets/1220815/f412cac5-2db6-43d0-9b02-d2269cb60824

We're able to keep track of where an override comes from, this is
exposed in the selection panels' tooltip:

![image](https://github.com/rerun-io/rerun/assets/1220815/ef7794ac-07c8-4930-ba92-11a45f91f6fe)



### 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/5508/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5508/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/5508/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/5508)
- [Docs
preview](https://rerun.io/preview/a09f77a2233fd101a4c5a94703f7726a920a18c6/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/a09f77a2233fd101a4c5a94703f7726a920a18c6/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
ui concerns graphical user interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants