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

Filter entities in the UI (part 3): Move action to a menu in the blueprint panel and keep default blueprint when using heuristics #8672

Merged
merged 8 commits into from
Jan 14, 2025

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Jan 13, 2025

Related

What

In preparation for adding the filter UI in the blueprint panel, we must move the current actions (add view/container, reset blueprint) to a "more" () icon menu. This in turn allows us to be explicit with the reset actions, and have both "reset to default" and "reset to heuristics" (like in the application's selection panel).

In turn, this exposes the weird behaviour where resetting to heuristics also deletes the current blueprint, for no reason other than it made implementation easier. This PR fixes that. So it's not possible to reset to heuristics and then go back to the default blueprint.

Also:

new-blueprint-panel-more-menu.mp4

@abey79 abey79 added ui concerns graphical user interface include in changelog labels Jan 13, 2025
Copy link

github-actions bot commented Jan 13, 2025

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

  • I have tested the web viewer
Result Commit Link Manifest
c5bc66b https://rerun.io/viewer/pr/8672 +nightly +main

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

@abey79 abey79 marked this pull request as ready for review January 13, 2025 20:37
@emilk emilk self-requested a review January 14, 2025 07:46
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.

Makes sense, but it is also all very confusing to me, because we are inconsistent in the naming.

We have the default blueprint (fine), and then we have the "automatic" "generated" "heuristic" blueprint. I wish we could settle on one name for it, instead of three.

crates/viewer/re_blueprint_tree/src/blueprint_tree.rs Outdated Show resolved Hide resolved
Comment on lines 35 to 36
/// Note: this engages the heuristics even if a default blueprint exists, so the default
/// blueprint may be restored.
Copy link
Member

Choose a reason for hiding this comment

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

I don't follow this. Why do we need to explicitly engage the heuristics to restore the default blueprint? Is the heuristic bools not part of the actual blueprint? If not, why aren't they reset automatically when there is no blueprint (i.e. after a reset)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Bad wording on my end, I meant to imply that the default blueprint (if any) is left alone, so the user may restore it later on. I've rewritten entirely that (and other) comment to clarify this.

if ui
.add_enabled(
enabled,
egui::Button::image_and_text(&re_ui::icons::RESET, "Reset to heuristic blueprint"),
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can come up with better copy here. "Create automatic blueprint"? Maybe @gavrelina has ideas

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 discussed that and decided to stick with "heuristics" for now.

@abey79
Copy link
Member Author

abey79 commented Jan 14, 2025

We have the default blueprint (fine), and then we have the "automatic" "generated" "heuristic" blueprint. I wish we could settle on one name for it, instead of three.

I tried to further cleaned that up. We should never use "generated". We agree to keep "heuristics" for user-facing stuff. We still have auto_layout/auto_view in the code. Maybe we should renamed those as well, but I'll keep that for another day.

@abey79 abey79 merged commit f779d8b into main Jan 14, 2025
31 checks passed
@abey79 abey79 deleted the antoine/filt3-blueprint-more-menu branch January 14, 2025 10:02
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