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 selection hierarchy breadcrumbs #8319

Merged
merged 42 commits into from
Dec 6, 2024
Merged

Add selection hierarchy breadcrumbs #8319

merged 42 commits into from
Dec 6, 2024

Conversation

emilk
Copy link
Member

@emilk emilk commented Dec 5, 2024

Related

What

This adds "breadcrumbs" to each selection header in the selection panel, showing the ancestry of the selected thing.
That should make it obvious if the selection belong to the blueprint tree panel, or the streams panel.

image
image
image
image

Future PR

  • Make projections appear the same in breadcrumbs as in the blueprint tree panel

@emilk emilk added ui concerns graphical user interface include in changelog labels Dec 5, 2024
Comment on lines +83 to 84
if entity_path.is_root() {
ui.strong("/");
Copy link
Member Author

Choose a reason for hiding this comment

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

We used to show nothing if the root was selected

Copy link

github-actions bot commented Dec 5, 2024

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link
53b5687 https://rerun.io/viewer/pr/8319

Note: This comment is updated whenever you push a commit.


let relative = &entity_path.as_slice()[common_ancestor.len()..];

let is_projection = !entity_path.starts_with(&view.space_origin);
Copy link
Member Author

Choose a reason for hiding this comment

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

I still haven't figured out how to properly match the breadcrumbs with the blueprint tree panel for projections, since I don't have a good mental model for how projections are shown in the blueprint tree panel.

@emilk emilk force-pushed the emilk/breadcrumbs branch from 8754d09 to 75bf9e6 Compare December 5, 2024 10:32
use crate::{DataResultNode, DataResultTree};

// TODO(@Wumpf): document this
pub enum DataResultNodeOrPath<'a> {
Copy link
Member Author

@emilk emilk Dec 5, 2024

Choose a reason for hiding this comment

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

@Wumpf you created this enum in #5342

I don't quite understand it, but I fear I may need to use of it in the next PR

@emilk emilk marked this pull request as ready for review December 5, 2024 12:05
@nikolausWest
Copy link
Member

Not sure I understood why we removed the "In view" row (@gavrelina)

@emilk
Copy link
Member Author

emilk commented Dec 5, 2024

Not sure I understood why we removed the "In view" row (@gavrelina)

I guess for the same reason as we don't have a "In container" row for views/containers: we can now see that from the breadcrumbs 🤷

@emilk emilk mentioned this pull request Dec 5, 2024
2 tasks
Copy link
Member

@abey79 abey79 left a comment

Choose a reason for hiding this comment

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

Thanks for that, very nice improvement!

Comment on lines +68 to +75
{
ui.spacing_mut().item_spacing.x = 2.0;

// Show one single icon up-front instead:
let instance_path = InstancePath::entity_all(entity_path.clone());
ui.add(instance_path_icon(&query.timeline(), db, &instance_path).as_image());
// The last part points to the selected entity, but that's ugly, so remove the highlight:
let visuals = ui.visuals_mut();
visuals.selection.bg_fill = egui::Color32::TRANSPARENT;
visuals.selection.stroke = visuals.widgets.inactive.fg_stroke;
}
Copy link
Member

Choose a reason for hiding this comment

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

does this scope serve a purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really - it is just a logical grouping, and reducing the scope of variables, thus lessoning the mental overhead

if instance.is_all() {
// Entity path. Exclude the last part from the breadcrumbs,
// as we will show it in full later on.
if let [all_but_last @ .., _] = entity_path.as_slice() {
Copy link
Member

Choose a reason for hiding this comment

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

woa TIL that syntax

Copy link
Member

Choose a reason for hiding this comment

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

really surprised this icon has odd values for both its dimensions. Shouldn't we aiming for good 2x ui scale factor?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

It's 4x8 in figma

image

Exporting it with 2x…
image

…results in a PNG that is 11x19
image

FIGMA YOU HIGH

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, could be because of the thickness of the line:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

I suggest we ignore this until we switch to .svg for all icons

crates/viewer/re_ui/src/list_item/list_item.rs Outdated Show resolved Hide resolved
@emilk emilk merged commit 003b00c into main Dec 6, 2024
31 checks passed
@emilk emilk deleted the emilk/breadcrumbs branch December 6, 2024 13:56
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.

3 participants