From ac9729170d3cb9278b56a45d74479013e3ddd067 Mon Sep 17 00:00:00 2001 From: ada mandala Date: Mon, 6 Nov 2023 11:50:21 -0600 Subject: [PATCH] Make edit button sticky for active expression cols Hide edit button for cols with no styles or attrs Hide the style tab unless needed (hack!) Add a size trap for column settings panel width Update column settings panel title to be less verbose Update tests --- .../src/rust/components/column_selector.rs | 10 +- .../column_selector/active_column.rs | 23 +++- .../column_selector/expression_toolbar.rs | 4 - .../column_settings_sidebar/sidebar.rs | 108 +++++++++++------- .../style_tab/column_style.rs | 2 +- .../style_tab/symbol.rs | 4 +- .../src/rust/components/containers/sidebar.rs | 35 +++++- .../src/rust/components/containers/tablist.rs | 15 ++- .../src/rust/components/viewer.rs | 1 + .../src/rust/model/plugin_config.rs | 20 ++-- .../test/js/column_settings.spec.ts | 37 ++---- tools/perspective-test/results.tar.gz | Bin 160599 -> 160473 bytes 12 files changed, 166 insertions(+), 93 deletions(-) diff --git a/rust/perspective-viewer/src/rust/components/column_selector.rs b/rust/perspective-viewer/src/rust/components/column_selector.rs index 7e5d4b1eca..85406e1e73 100644 --- a/rust/perspective-viewer/src/rust/components/column_selector.rs +++ b/rust/perspective-viewer/src/rust/components/column_selector.rs @@ -269,9 +269,15 @@ impl Component for ColumnSelector { let ondragenter = ondragenter.reform(move |_| Some(idx)); let size_hint = if named_count > 0 { 50.0 } else { 28.0 }; named_count = named_count.saturating_sub(1); - let key = name.get_name().map(|x| x.to_owned()).unwrap_or_else(|| format!("__auto_{}__", idx)); + let key = name + .get_name() + .map(|x| x.to_owned()) + .unwrap_or_else(|| format!("__auto_{}__", idx)); let column_dropdown = self.column_dropdown.clone(); - let is_editing = matches!(&ctx.props().selected_column, Some(ColumnLocator::Plain(x)) if x == &key); + let is_editing = matches!( + &ctx.props().selected_column, + Some(ColumnLocator::Plain(x)) | Some(ColumnLocator::Expr(Some(x))) if x == &key + ); html_nested! { col_type != Type::Bool, + "X/Y Scatter" => col_type == Type::String && label.as_deref() == Some("Symbol"), + _ => false, + } || is_expression; + html! {
} - + if show_edit_btn { + + }
diff --git a/rust/perspective-viewer/src/rust/components/column_selector/expression_toolbar.rs b/rust/perspective-viewer/src/rust/components/column_selector/expression_toolbar.rs index a6394ef449..ac7080357c 100644 --- a/rust/perspective-viewer/src/rust/components/column_selector/expression_toolbar.rs +++ b/rust/perspective-viewer/src/rust/components/column_selector/expression_toolbar.rs @@ -22,10 +22,6 @@ pub struct ExprEditButtonProps { pub is_editing: bool, } -// TODO: Move this logic to ColumnSettingsSidebar -// Button should just pass the name to the on_open callback, and the sidebar -// should render it. generally, rendering logic should live on the closest -// component /// A button that goes into a column-list for a custom expression /// when pressed, it opens up the expression editor side-panel. #[function_component] diff --git a/rust/perspective-viewer/src/rust/components/column_settings_sidebar/sidebar.rs b/rust/perspective-viewer/src/rust/components/column_settings_sidebar/sidebar.rs index ef0d5f75bf..4c11047f89 100644 --- a/rust/perspective-viewer/src/rust/components/column_settings_sidebar/sidebar.rs +++ b/rust/perspective-viewer/src/rust/components/column_settings_sidebar/sidebar.rs @@ -12,7 +12,7 @@ use std::fmt::Display; -use yew::{function_component, html, Callback, Html, Properties}; +use yew::{function_component, html, use_callback, use_state, Callback, Html, Properties}; use crate::components::column_settings_sidebar::attributes_tab::AttributesTab; use crate::components::column_settings_sidebar::style_tab::StyleTab; @@ -21,6 +21,7 @@ use crate::components::containers::tablist::{Tab, TabList}; use crate::components::expression_editor::get_new_column_name; use crate::components::style::LocalStyle; use crate::components::viewer::ColumnLocator; +use crate::config::Type; use crate::custom_events::CustomEvents; use crate::model::*; use crate::renderer::Renderer; @@ -49,6 +50,7 @@ pub struct ColumnSettingsProps { pub session: Session, pub renderer: Renderer, pub custom_events: CustomEvents, + pub width_override: Option, } derive_model!(CustomEvents, Session, Renderer for ColumnSettingsProps); @@ -69,7 +71,7 @@ pub fn ColumnSettingsSidebar(p: &ColumnSettingsProps) -> Html { let column_type = p.session.metadata().get_column_view_type(&column_name); let is_active = column_type.is_some(); - let (config, attrs) = p.get_plugin_config(); + let (config, attrs) = (p.get_plugin_config(), p.get_plugin_attrs()); if config.is_none() || attrs.is_none() { tracing::warn!( "Could not get full plugin config!\nconfig (plugin.save()): {:?}\nplugin_attrs: {:?}", @@ -79,14 +81,22 @@ pub fn ColumnSettingsSidebar(p: &ColumnSettingsProps) -> Html { } let maybe_ty = p.session.metadata().get_column_view_type(&column_name); - let title = format!("Editing ‘{column_name}’..."); - let mut tabs = vec![]; + // TODO: This is a hack and needs to be replaced. + let plugin = p.renderer.get_active_plugin().unwrap(); + let show_styles = maybe_ty + .map(|ty| match &*plugin.name() { + "Datagrid" => ty != Type::Bool, + "X/Y Scatter" => ty == Type::String, + _ => false, + }) + .unwrap_or_default(); + if !matches!(p.selected_column, ColumnLocator::Expr(None)) + && show_styles && is_active && config.is_some() - && attrs.is_some() && maybe_ty.is_some() { tabs.push(ColumnSettingsTab::Style); @@ -95,58 +105,76 @@ pub fn ColumnSettingsSidebar(p: &ColumnSettingsProps) -> Html { tabs.push(ColumnSettingsTab::Attributes); } - clone!( - p.selected_column, - p.on_close, - p.session, - p.renderer, - p.custom_events, - column_name - ); - let match_fn = Callback::from(move |tab| { + let match_fn = { clone!( - selected_column, - on_close, - session, - renderer, - custom_events, + p.selected_column, + p.on_close, + p.session, + p.renderer, + p.custom_events, column_name ); - match tab { - ColumnSettingsTab::Attributes => { - html! { - { + html! { + + } + } + ColumnSettingsTab::Style => html! { + - } + }, } - ColumnSettingsTab::Style => html! { - - }, - } - }); + }) + }; + + let selected_tab = use_state(|| None); + + let on_tab_change = { + clone!(selected_tab); + use_callback((), move |idx, _| { + selected_tab.set(Some(idx)); + }) + }; html_template! { - {tabs} {match_fn} /> + + {tabs} + {match_fn} + {on_tab_change} + selected_tab={*selected_tab} + /> } diff --git a/rust/perspective-viewer/src/rust/components/column_settings_sidebar/style_tab/column_style.rs b/rust/perspective-viewer/src/rust/components/column_settings_sidebar/style_tab/column_style.rs index ea80734e7d..d9b8a2a0d8 100644 --- a/rust/perspective-viewer/src/rust/components/column_settings_sidebar/style_tab/column_style.rs +++ b/rust/perspective-viewer/src/rust/components/column_settings_sidebar/style_tab/column_style.rs @@ -76,7 +76,7 @@ derive_model!(CustomEvents, Session, Renderer for ColumnStyleProps); #[function_component] pub fn ColumnStyle(p: &ColumnStyleProps) -> Html { let props = p.clone(); - let (config, attrs) = props.get_plugin_config(); + let (config, attrs) = (props.get_plugin_config(), props.get_plugin_attrs()); let (config, attrs) = (config.unwrap(), attrs.unwrap()); let title = format!("{} Styling", props.ty.to_capitalized()); let opt_html = match props.ty { diff --git a/rust/perspective-viewer/src/rust/components/column_settings_sidebar/style_tab/symbol.rs b/rust/perspective-viewer/src/rust/components/column_settings_sidebar/style_tab/symbol.rs index d8148435c5..58d72b007e 100644 --- a/rust/perspective-viewer/src/rust/components/column_settings_sidebar/style_tab/symbol.rs +++ b/rust/perspective-viewer/src/rust/components/column_settings_sidebar/style_tab/symbol.rs @@ -27,7 +27,7 @@ use crate::config::plugin::{PluginConfig, Symbol}; use crate::config::Type; use crate::custom_elements::FilterDropDownElement; use crate::custom_events::CustomEvents; -use crate::model::UpdatePluginConfig; +use crate::model::{GetPluginConfig, UpdatePluginConfig}; use crate::renderer::Renderer; use crate::session::Session; use crate::{css, derive_model, html_template}; @@ -49,7 +49,7 @@ pub struct SymbolAttrProps { derive_model!(CustomEvents, Session, Renderer for SymbolAttrProps); impl SymbolAttrProps { pub fn get_config(&self) -> (PluginConfig, Vec) { - let (config, attrs) = self.get_plugin_config(); + let (config, attrs) = (self.get_plugin_config(), self.get_plugin_attrs()); ( config.unwrap(), attrs.and_then(|a| a.symbol.map(|s| s.symbols)).unwrap(), diff --git a/rust/perspective-viewer/src/rust/components/containers/sidebar.rs b/rust/perspective-viewer/src/rust/components/containers/sidebar.rs index 5f2deacfc0..88127a820c 100644 --- a/rust/perspective-viewer/src/rust/components/containers/sidebar.rs +++ b/rust/perspective-viewer/src/rust/components/containers/sidebar.rs @@ -10,7 +10,13 @@ // ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ -use yew::{function_component, html, AttrValue, Callback, Children, Html, Properties}; +use web_sys::Element; +use yew::{ + function_component, html, use_effect_with, use_node_ref, use_state_eq, AttrValue, Callback, + Children, Html, Properties, +}; + +use crate::clone; #[derive(PartialEq, Clone, Properties)] pub struct SidebarProps { @@ -21,14 +27,38 @@ pub struct SidebarProps { pub title: String, pub id_prefix: String, pub icon: Option, + pub width_override: Option, + pub selected_tab: Option, } /// Sidebars are designed to live in a [SplitPanel] #[function_component] pub fn Sidebar(p: &SidebarProps) -> Html { let id = &p.id_prefix; + let noderef = use_node_ref(); + let auto_width = use_state_eq(|| 0f64); + + // this gets the last calculated width and ensures that + // the auto-width element is at least that big. + // this ensures the panel grows but does not shrink. + use_effect_with((p.children.clone(), p.selected_tab), { + clone!(noderef, auto_width, p.width_override); + move |_| { + if width_override.is_none() { + let updated_width = noderef + .cast::() + .map(|el| el.get_bounding_client_rect().width()) + .unwrap_or_default(); + auto_width.set((*auto_width).max(updated_width)); + } else { + auto_width.set(0f64); + } + } + }); + let width_style = format!("min-width: 200px; width: {}px", *auto_width); + html! { -