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

Show all entities/components in the Streams UI, even if empty for the selected timeline #3779

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Oct 10, 2023

What

Before, the Streams UI would only include those entities/components that either have data logged on the currently selected timeline, or timeless data logged. This means that depending on the user code, no data could end up being displayed, or the tree content could vary depending on the selected timeline.

This PR changes the behaviour such that all entities/components are now always displayed in the Streams UI, and it adds a conspicuous note in the tooltips to indicate why the timeline might be empty.

image

This is a first step in addressing #3733. A follow-up PR should further enhance the styling of hidden entities/components to make it more obvious that they are "timeline-shadowed", and apply the same to the Blueprints Tree UI.

Partially addresses:

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 include in changelog labels Oct 10, 2023
@abey79 abey79 force-pushed the antoine/show-streams-wo-data-3733 branch from 73e96c7 to f8edf1b Compare October 10, 2023 12:57
crates/re_data_ui/src/instance_path.rs Outdated Show resolved Hide resolved
Comment on lines 420 to 423
let tree_has_data_in_current_timeline = tree
.prefix_times
.has_timeline(ctx.rec_cfg.time_ctrl.timeline())
&& tree.num_timeless_messages() == 0
{
return; // ignore entities that have no data for the current timeline, nor any timeless data.
}
|| tree.num_timeless_messages() > 0;
Copy link
Member

Choose a reason for hiding this comment

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

It's great that you made a readable variable out of this, but let's take it to the logical conclusion and make helper function for this: tree.has_data_in_current_timeline or similar

Copy link
Member

Choose a reason for hiding this comment

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

Also note that this will return true for world if data is logged to world/point, since world is a prefix of world/point

Copy link
Member Author

Choose a reason for hiding this comment

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

Also note that this will return true for world if data is logged to world/point, since world is a prefix of world/point

Yeah, that's the desired behaviour for reporting "emptiness" I think.

Comment on lines 547 to 549
let component_has_data_in_current_timeline =
data.times.has_timeline(ctx.rec_cfg.time_ctrl.timeline())
|| data.num_timeless_messages() > 0;
Copy link
Member

Choose a reason for hiding this comment

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

again, this could be turned into a helper function for even more increased readability (even though this is already an improvement)

@abey79
Copy link
Member Author

abey79 commented Oct 10, 2023

@emilk The annoying thing with those helper function is that they require stuff from very different places inside ViewerContext (ctx.rec_cfg... and the tree/component). I've opted to put those helper at the top level of ViewerContext so that they are nice to use, but it feels slightly random—let me know if you think there is a better way.

My follow-up PR would quite naturally add a entity_path_has_data_in_current_timeline() btw.

Also, note that in show_[tree|children], I call the helper kind of far from the use site. The reason is that my follow-up PR will need that for styling of the ListItem.

@abey79 abey79 merged commit 68e4d2e into main Oct 11, 2023
30 of 32 checks passed
@abey79 abey79 deleted the antoine/show-streams-wo-data-3733 branch October 11, 2023 06:58
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.

2 participants