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 Recordings section to the left panel #2938

Merged
merged 13 commits into from
Aug 11, 2023
Merged

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Aug 8, 2023

What

Add Recordings section to the left panel.

Fixes #2298

Also:

  • created a brand new ListItem widget to be used across the UI
    • full-span highlight
    • icon support (Icon or closure)
    • hover button support
  • created a proper "panel_title_bar" widget in ReUi
  • removed the recordings submenu from the rerun menu
  • updated re_ui_example.rs to illustrate the "proper" panel hierarchy and demo the new widgets
  • + button in title to open new recording

Deferred to follow-up PRs:

  • hierarchical display
  • minus hover button to close the recording
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) best tested on a native build and throwing lots of example at it, e.g. for run in {1..4}; do python examples/python/dna/main.py --connect; done

abey79 added 3 commits August 8, 2023 16:06
- created a proper "panel_title_bar" widget in ReUi
- removed the recordings submenu from the rerun menu
- updated re_ui_example.rs to illustrate the "proper" panel hierarchy and demo the "panel_title_bar" widget.
@abey79 abey79 added the ui concerns graphical user interface label Aug 8, 2023
abey79 added 6 commits August 9, 2023 18:57
Note: scroll area breaks the clip rect :(
- recordings now use ListItem and have proper icon
- new "canonical" panel hierarchy defined—again!
- ListItem has custom icon function now
- panel_content() helper function (for the panel frame with inner margins)
@abey79 abey79 changed the title (WIP) Add Recordings section to the left panel Add Recordings section to the left panel Aug 10, 2023
@abey79 abey79 marked this pull request as ready for review August 10, 2023 15:27
@abey79 abey79 requested a review from martenbjork August 10, 2023 15:35
Copy link
Contributor

@martenbjork martenbjork left a comment

Choose a reason for hiding this comment

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

Super! 🔥

@emilk
Copy link
Member

emilk commented Aug 10, 2023

When I use the + button to open new recording, the newly open recording is not automatically made the selected once

@abey79
Copy link
Member Author

abey79 commented Aug 10, 2023

When I use the + button to open new recording, the newly open recording is not automatically made the selected once

So does the "Open..." menu item then? It should be doing the same thing. Anyway, I'll take a look tomorrow.

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.

Looks good! Nothing major to interject.

Feel free to merge once my comments has been addressed (or ignored as you see fit)

crates/re_log_types/src/time.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
// Second section. It's a list of `list_items`, so we need to remove the default
// spacing. Also, it uses a scroll area, so we must use several `Frame`s.
ui.scope(|ui| {
ui.spacing_mut().item_spacing.y = 0.0;
Copy link
Member

Choose a reason for hiding this comment

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

so we want to remove the spacing between the title and the items too?

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 think so, though the styling of the title has already changed in the meantime and that is all part of a whole lot of things that will see significant styling adjustment and fine-tuning in the coming weeks (when Mårten is back from holidays).

crates/re_ui/src/lib.rs Outdated Show resolved Hide resolved
crates/re_ui/src/list_item.rs Show resolved Hide resolved
crates/re_viewer/src/ui/recordings_panel.rs Outdated Show resolved Hide resolved
crates/re_viewer/src/ui/recordings_panel.rs Outdated Show resolved Hide resolved
crates/re_viewer/src/ui/recordings_panel.rs Outdated Show resolved Hide resolved
crates/re_viewer/src/ui/recordings_panel.rs Outdated Show resolved Hide resolved
crates/re_viewer/src/ui/rerun_menu.rs Outdated Show resolved Hide resolved
@emilk
Copy link
Member

emilk commented Aug 10, 2023

When I use the + button to open new recording, the newly open recording is not automatically made the selected once

So does the "Open..." menu item then? It should be doing the same thing. Anyway, I'll take a look tomorrow.

Yeah you are right, probably not a new bug, just more obvious now (which is a good thing!)

@abey79
Copy link
Member Author

abey79 commented Aug 11, 2023

@emilk Great review, thanks! (I learned a bunch on the way.)

I've addressed all the comments marked as resolved. I guess the ceil() is the remaining thing to clarify.

I also fixed the "loaded rrd isn't activated" bug. Maybe you'll want to take a look as I haven't touched that stuff much so far.

crates/re_ui/src/list_item.rs Outdated Show resolved Hide resolved
@abey79 abey79 merged commit 4f2fe7d into main Aug 11, 2023
@abey79 abey79 deleted the antoine/new-recording-ui-2298 branch August 11, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show open recordings in side view above blueprints
3 participants