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

Improve the Selection Panel with better title, context, and Space View key properties #4324

Merged
merged 6 commits into from
Nov 27, 2023

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Nov 23, 2023

What

Do not merge until after 0.11.

This PR reworks the Selection Panel to improve the title ("What is selected?"), the context ("Where is the selected thing to be found?"), and the key properties of Space Views (name, origin, type). This is a first step towards the WIP designs by @martenbjork related to blueprints.

Changes:

  • Full-width, blue title bar consistent with other panel's style and with the idea that this thing is selected.
  • "Context" section listing the place(s) this thing is to be found.
    • This is much more consistant across Item kinds. Previously, stuff was displayed all over the place.
    • From StreamEntity, it's now it's possible to navigate to any related SpaceViewEntity or Space Views. Previously, a button confusingly named after the space view would actually lead to the SpaceViewEntity.
  • For Space Views, the Name/Origin/Type are now displayed on top (for all types of space views).
    • This greatly increase the discoverability of the "Name" property. Previously it was hiding in plain sight in what appeared to be contextual information.

Note: the end-game for context is bread-crumbs like these, but that's for another day as we're don't yet have an easy way to walk up the hierarchy:

image

Screenshots

Space View:
image

Group:

image

Entity in Space View:

image

Entity in Streams:

image

Component:

image

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 demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@abey79 abey79 added ui concerns graphical user interface do-not-merge Do not merge this PR include in changelog labels Nov 23, 2023
@nikolausWest
Copy link
Member

nikolausWest commented Nov 23, 2023

From a first test, I think it's a pretty straight forward improvement over what we have now. I guess the only thing that is a slight negative is that we no longer write out what the type of the thing you've selected is. Since this info exists on hover and we intend to indicate the type also through a new set of icons in the future I don't think that should be a blocker.

I would just ship this as-is. Good stuff!

@abey79
Copy link
Member Author

abey79 commented Nov 24, 2023

@nikolausWest I don't disagree. As a matter of fact, my "design" included explicit type:

image

We shortly discussed that with @martenbjork who preferred to not overload this title bar. We can certainly revisit this and iterate.

@emilk
Copy link
Member

emilk commented Nov 24, 2023

Do not merge until after 0.11.

why not?

@abey79
Copy link
Member Author

abey79 commented Nov 24, 2023

why not?

I was seeing this as a first step towards a bigger story, but yeah if we like it we can roll it in 0.11—no technical reason not to. We probably shouldn't delay 0.11 for this though.

@abey79 abey79 removed the do-not-merge Do not merge this PR label Nov 24, 2023
@emilk
Copy link
Member

emilk commented Nov 27, 2023

I don't know if this is new, but the existence or not of scroll bars causes the width of the header fills to change:

width-bug

@abey79
Copy link
Member Author

abey79 commented Nov 27, 2023

@emilk This is not new, but got more visible with the blue background. I had hoped the floating scroll bars would magically fix it, but it's apparently not the case. I will look into it—at minima, open an issue if we dont have one.

edit: fixed by #4340

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.

I think the // NOLINT is a hint that we're doing something wrong, but otherwise LGTM

crates/re_space_view_spatial/src/ui.rs Outdated Show resolved Hide resolved
crates/re_viewer/src/ui/selection_panel.rs Show resolved Hide resolved
crates/re_viewer/src/ui/selection_panel.rs Outdated Show resolved Hide resolved
@abey79
Copy link
Member Author

abey79 commented Nov 27, 2023

I just noticed this PR makes a pre-existing visual glitch way more apparent, see the blue background shrinking when the scroll bar appears.

Export-1701081583965

I believe this is not fixed by #4340, though worth rebasing and checking when it's merged. Nvm, this should be fixed by #4340 now.

abey79 added a commit that referenced this pull request Nov 27, 2023
### What

This PR changes the way inner margins and `ScrollAreas` are nested to
avoid some visual issues.

- Fixes #3165
- Fixes full span display bug:
#4324 (comment)

### 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 [demo.rerun.io](https://demo.rerun.io/pr/4340) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4340)
- [Docs
preview](https://rerun.io/preview/71996e8cabf9dc864f4d7240779b3271472876c3/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/71996e8cabf9dc864f4d7240779b3271472876c3/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@Wumpf Wumpf merged commit 7789517 into main Nov 27, 2023
39 checks passed
@Wumpf Wumpf deleted the antoine/selection-panel-top-rework branch November 27, 2023 14:40
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
4 participants