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

Inline viewer improvements #6504

Merged
merged 15 commits into from
Jun 6, 2024
Merged
2 changes: 1 addition & 1 deletion .prettierrc.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ files = ["*.yml", "*.yaml"]
options = { tabWidth = 2 }

[[overrides]]
files = ["*.js", "*.mjs", "*.cjs"]
files = ["*.js", "*.mjs", "*.cjs", "*.ts", "*.d.ts"]
options = { semi = true, tabWidth = 2 }

[[overrides]]
Expand Down
12 changes: 12 additions & 0 deletions crates/re_viewer/src/app_blueprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,18 @@ impl<'a> AppBlueprint<'a> {
command_sender,
);
}

pub fn blueprint_panel_overridden(&self) -> bool {
self.overrides.is_some_and(|s| s.blueprint.is_some())
}

pub fn selection_panel_overridden(&self) -> bool {
self.overrides.is_some_and(|s| s.selection.is_some())
}

pub fn time_panel_overridden(&self) -> bool {
self.overrides.is_some_and(|s| s.time.is_some())
}
}

#[derive(Debug, Default, Clone, Copy)]
Expand Down
68 changes: 35 additions & 33 deletions crates/re_viewer/src/ui/top_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,14 @@ fn connection_status_ui(ui: &mut egui::Ui, rx: &ReceiveSet<re_log_types::LogMsg>
fn panel_buttons_r2l(app: &App, app_blueprint: &AppBlueprint<'_>, ui: &mut egui::Ui) {
#[cfg(target_arch = "wasm32")]
if app.is_fullscreen_allowed() {
let mut is_fullscreen = app.is_fullscreen_mode();
let icon = if is_fullscreen {
let icon = if app.is_fullscreen_mode() {
&re_ui::icons::MINIMIZE
} else {
&re_ui::icons::MAXIMIZE
};

if ui
.medium_icon_toggle_button(icon, &mut is_fullscreen)
.medium_icon_toggle_button(icon, &mut true)
.on_hover_text("Toggle fullscreen")
.clicked()
{
Expand All @@ -261,46 +260,49 @@ fn panel_buttons_r2l(app: &App, app_blueprint: &AppBlueprint<'_>, ui: &mut egui:
}

// selection panel
if ui
.medium_icon_toggle_button(
&re_ui::icons::RIGHT_PANEL_TOGGLE,
&mut app_blueprint.selection_panel_state().is_expanded(),
)
.on_hover_text(format!(
"Toggle Selection View{}",
UICommand::ToggleSelectionPanel.format_shortcut_tooltip_suffix(ui.ctx())
))
.clicked()
if !app_blueprint.selection_panel_overridden()
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR since the logic probably adds too much complexity, but I think we may want to reconsider this at some point and have this be something like: !app_blueprint.selection_panel_hidden()

It would be nice if the state was determine by:

  • If hidden, never show it -- no UI controls.
  • If user has interacted with the controls, use that value.
  • If overridden from JS, apply that value.
  • If the blueprint has a value, apply that value.

&& ui
.medium_icon_toggle_button(
&re_ui::icons::RIGHT_PANEL_TOGGLE,
&mut app_blueprint.selection_panel_state().is_expanded(),
)
.on_hover_text(format!(
"Toggle Selection View{}",
UICommand::ToggleSelectionPanel.format_shortcut_tooltip_suffix(ui.ctx())
))
.clicked()
{
app_blueprint.toggle_selection_panel(&app.command_sender);
}

// time panel
if ui
.medium_icon_toggle_button(
&re_ui::icons::BOTTOM_PANEL_TOGGLE,
&mut app_blueprint.time_panel_state().is_expanded(),
)
.on_hover_text(format!(
"Toggle Timeline View{}",
UICommand::ToggleTimePanel.format_shortcut_tooltip_suffix(ui.ctx())
))
.clicked()
if !app_blueprint.time_panel_overridden()
&& ui
.medium_icon_toggle_button(
&re_ui::icons::BOTTOM_PANEL_TOGGLE,
&mut app_blueprint.time_panel_state().is_expanded(),
)
.on_hover_text(format!(
"Toggle Timeline View{}",
UICommand::ToggleTimePanel.format_shortcut_tooltip_suffix(ui.ctx())
))
.clicked()
{
app_blueprint.toggle_time_panel(&app.command_sender);
}

// blueprint panel
if ui
.medium_icon_toggle_button(
&re_ui::icons::LEFT_PANEL_TOGGLE,
&mut app_blueprint.blueprint_panel_state().is_expanded(),
)
.on_hover_text(format!(
"Toggle blueprint view{}",
UICommand::ToggleBlueprintPanel.format_shortcut_tooltip_suffix(ui.ctx())
))
.clicked()
if !app_blueprint.blueprint_panel_overridden()
&& ui
.medium_icon_toggle_button(
&re_ui::icons::LEFT_PANEL_TOGGLE,
&mut app_blueprint.blueprint_panel_state().is_expanded(),
)
.on_hover_text(format!(
"Toggle blueprint view{}",
UICommand::ToggleBlueprintPanel.format_shortcut_tooltip_suffix(ui.ctx())
))
.clicked()
{
app_blueprint.toggle_blueprint_panel(&app.command_sender);
}
Expand Down
10 changes: 8 additions & 2 deletions crates/re_viewer/src/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,18 @@ impl WebHandle {
}

#[wasm_bindgen]
pub fn toggle_panel_overrides(&self) {
pub fn toggle_panel_overrides(&self, value: Option<bool>) {
Copy link
Member

Choose a reason for hiding this comment

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

I find the existence of this API as a toggle (as opposed to just something like set_panel_overrides_active) to be a bit unexpected.

I'm glad to see us turning it into a hybrid set/toggle, though the name as toggle still seems like it might hurt discoverability / expectation somewhat.

let Some(mut app) = self.runner.app_mut::<crate::App>() else {
return;
};

app.panel_state_overrides_active ^= true;
match value {
Some(value) => app.panel_state_overrides_active = value,
None => app.panel_state_overrides_active ^= true,
}

// request repaint, because the overrides may cause panels to expand/collapse
app.egui_ctx.request_repaint();
}

#[wasm_bindgen]
Expand Down
2 changes: 2 additions & 0 deletions rerun_js/scripts/common.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ export function inferTag(version) {
switch (kind) {
case "alpha":
tag = "alpha";
break;
case "rc":
tag = "rc";
break;
}
}

Expand Down
2 changes: 2 additions & 0 deletions rerun_js/web-viewer/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ re_viewer_bg.wasm
re_viewer_bg.wasm.d.ts
index.d.ts
index.d.ts.map
index.js
index.js.map
Loading
Loading