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

Allow hiding top panel via blueprint #6409

Merged
merged 10 commits into from
May 27, 2024
Merged

Conversation

jprochazk
Copy link
Member

@jprochazk jprochazk commented May 22, 2024

What

  • rrb.TopPanel is now exposed, works similarly to rrb.TimePanel
  • TimePanel hidden state now fully hides the time panel

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 the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@jprochazk jprochazk added 🐍 Python API Python logging API ui concerns graphical user interface 🟦 blueprint The data that defines our UI include in changelog labels May 22, 2024
@jprochazk jprochazk requested a review from Wumpf May 22, 2024 18:58
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

doesn't actually work ;)

Bunch of things we need to do:

  • process the incoming TopPanel
  • extend docs:
    • what does collapsed vs hidden mean on each of the panels
    • what does collapse_all thingy bool actually do now
    • ... stuff?

How I tested:
pixi run python ./docs/snippets/all/views/spatial2d.py
pixi run ipython
then

import rerun.blueprint as rrb
rrb.Blueprint(rrb.TopPanel(state="collapsed")).connect("rerun_example_spatial_2d")

@@ -884,7 +884,7 @@ impl App {

crate::ui::mobile_warning_ui(&self.re_ui, ui);

if app_blueprint.top_panel_state != PanelState::Hidden {
if app_blueprint.top_panel_state == PanelState::Expanded {
Copy link
Member Author

Choose a reason for hiding this comment

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

this is to align with behavior in #6419

@jprochazk jprochazk requested a review from Wumpf May 23, 2024 12:54
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

On native mac the hidden top panel doesn't work out, I think we have to do a per-platform thing here

when selected:
image

when unselected:
image

I figure on Windows and Linux this is fine because we don't mange the close buttons there. Also, for web this is obviously a good feature to have (maybe actually worth pointing out in the doc that this is useful for web mostly).
I'd propose to disable this on Mac (needs to be documented) and makes sure this looks alright on Windows & Linux.

rerun_py/rerun_sdk/rerun/blueprint/api.py Outdated Show resolved Hide resolved
rerun_py/rerun_sdk/rerun/blueprint/api.py Outdated Show resolved Hide resolved
@jprochazk
Copy link
Member Author

jprochazk commented May 27, 2024

I'd propose to disable this on Mac (needs to be documented) and makes sure this looks alright on Windows & Linux.

Instead of disabling it, I made the top panel toggle hide its content instead of the full thing on mac. Currently making sure it looks alright everywhere:

  • Windows
  • Mac
  • Linux

@Wumpf Wumpf self-requested a review May 27, 2024 11:05
@Wumpf
Copy link
Member

Wumpf commented May 27, 2024

good idea! Confirmed on my side this looks alright on Mac

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

At first I thought "fully hides the panel" isn't quite right on Mac now, but arguably it still is as we only have the bare minimum title bar that the OS needs essentially (we could shrink it a bit down with only having the mac buttons in there but whatever, can do that later if we really want to optimize this), so let's keep things simple just as your PR suggests

Great stuff and thanks for all the followups!

@jprochazk jprochazk merged commit 30ec825 into main May 27, 2024
35 checks passed
@jprochazk jprochazk deleted the jan/top-panel-in-app-blueprint branch May 27, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🟦 blueprint The data that defines our UI include in changelog 🐍 Python API Python logging API ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants