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.

Sign up for GitHub

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

Name space views after the space and indicate duplicate names #1653

Merged
merged 3 commits into from
Mar 21, 2023
Merged

Name space views after the space and indicate duplicate names #1653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb3cb4a
Always name the view after the base of the space path
jleibs Mar 21, 2023
e36d86c
Make space view name unique when adding to blueprint
jleibs Mar 21, 2023
ca238b9
Update comment
jleibs Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions crates/re_viewer/src/ui/space_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,11 @@ impl SpaceView {
space_path: &EntityPath,
queries_entities: &[EntityPath],
) -> Self {
let display_name = if queries_entities.len() == 1 {
// A single entity in this space-view - name the space after it.
queries_entities[0]
.last()
.map_or_else(|| "/".to_owned(), |part| part.to_string())
} else if let Some(name) = space_path.iter().last() {
// We previously named the [`SpaceView`] after the [`EntityPath`] if there was only a single entity. However,
// this led to somewhat confusing and inconsistent behavior. See https://github.com/rerun-io/rerun/issues/1220
// Spaces are now always named after the final element of the space-path (or the root), independent of the
// query entities.
let display_name = if let Some(name) = space_path.iter().last() {
Copy link
Member

Choose a reason for hiding this comment

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

With this PR we now sometimes get more things named after root (/):

image

…but I still think that this PR is an improvement.

But it would be good to add a comment here for why this is the better choice, so we don't regress here.

name.to_string()
} else {
// Include category name in the display for root paths because they look a tad bit too short otherwise.
Expand Down
20 changes: 19 additions & 1 deletion crates/re_viewer/src/ui/viewport.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,26 @@ impl Viewport {
self.has_been_user_edited = true;
}

pub(crate) fn add_space_view(&mut self, space_view: SpaceView) -> SpaceViewId {
pub(crate) fn add_space_view(&mut self, mut space_view: SpaceView) -> SpaceViewId {
let id = space_view.id;

// Find a unique name for the space view
let mut candidate_name = space_view.display_name.clone();
let mut append_count = 1;
let unique_name = 'outer: loop {
for view in &self.space_views {
if candidate_name == view.1.display_name {
append_count += 1;
candidate_name = format!("{} ({})", space_view.display_name, append_count);

continue 'outer;
}
}
break candidate_name;
};

space_view.display_name = unique_name;

self.space_views.insert(id, space_view);
self.visible.insert(id);
self.trees.clear(); // Reset them
Expand Down