Merge pull request #2864 from finos/workspace-dark-theme

Fix workspace dark theme, theme render bug

Author: texodus

packages/perspective-workspace/src/less/menu.less

@@ -19,8 +19,8 @@
 .lm-Menu {
     font-size: 12px;
     padding: 8px;
-    background: white;
-    color: #666;
+    background-color: var(--plugin--background);
+    color: var(--icon--color);
     border: 1px solid var(--inactive--color);
     border-radius: 3px;
     max-width: 350px;

packages/perspective-workspace/src/themes/pro-dark.less

@@ -22,9 +22,6 @@ perspective-indicator[theme="Pro Dark"] {
     --theme-name: "Pro Dark";
 }
 
-perspective-workspace-menu {
-    @include perspective-viewer-pro-dark--colors;
-}
 
 perspective-workspace perspective-viewer {
     --status-bar--height: 38px;

packages/perspective-workspace/src/themes/pro.less

@@ -24,11 +24,6 @@ perspective-workspace {
     background-color: #dadada;
 }
 
-perspective-workspace-menu {
-    @include perspective-viewer-pro--colors;
-    background-color: #ffffff;
-}
-
 perspective-workspace perspective-viewer[settings] {
     --modal-panel--margin: -4px 0 -4px 0;
     --status-bar--border-radius: 6px 0 0 0;

packages/perspective-workspace/src/ts/workspace/commands.ts

@@ -234,11 +234,11 @@ export const createCommands = (
             workspace.toggleMasterDetail(
                 workspace.getWidgetByName(args.widget_name as string)!
             ),
-        isVisible: () => true,
-        // iconClass: (args) =>
-        //     args.widget.parent === workspace.dockpanel
-        //         ? "menu-master"
-        //         : "menu-detail",
+        isVisible: (args) => {
+            return !!workspace.getWidgetByName(args.widget_name as string)
+                ?._is_pivoted;
+        },
+
         label: (args) => {
             return workspace.getWidgetByName(args.widget_name as string)!
                 .parent === workspace.get_dock_panel()

packages/perspective-workspace/src/ts/workspace/tabbar.ts

@@ -129,11 +129,12 @@ export class PerspectiveTabBar extends TabBar<any> {
         });
 
         const box = (event.target as HTMLElement).getBoundingClientRect();
-        this._menu.open(box.x, box.y + box.height);
-
+        const outer_box = this._workspace.element.getBoundingClientRect();
+        this._menu.open(box.x - outer_box.x, box.y + box.height - outer_box.y);
         this._menu.aboutToClose.connect(() => {
             this._menu = undefined;
         });
+
         event.preventDefault();
         event.stopPropagation();
     }

packages/perspective-workspace/src/ts/workspace/widget.ts

@@ -25,6 +25,7 @@ export class PerspectiveViewerWidget extends Widget {
     viewer: psp_viewer.HTMLPerspectiveViewerElement;
     _title: string;
     _is_table_loaded: boolean;
+    _is_pivoted: boolean;
     _restore_config?: () => Promise<void>;
     task?: Promise<void>;
 
@@ -33,6 +34,7 @@ export class PerspectiveViewerWidget extends Widget {
         this.viewer = viewer;
         this._title = "";
         this._is_table_loaded = false;
+        this._is_pivoted = false;
     }
 
     get name(): string {
@@ -65,16 +67,7 @@ export class PerspectiveViewerWidget extends Widget {
             this.viewer.setAttribute("table", table);
         }
 
-        // if (selectable) {
-        //     this.viewer.setAttribute("selectable", selectable);
-        // }
-
-        // if (editable) {
-        //     this.viewer.setAttribute("editable", editable);
-        // }
-
         const restore_config = () => this.viewer.restore({ ...viewerConfig });
-
         if (this._is_table_loaded) {
             return restore_config();
         } else {

packages/perspective-workspace/src/ts/workspace/workspace.ts

@@ -53,7 +53,7 @@ export class PerspectiveWorkspace extends SplitPanel {
     private dockpanel: PerspectiveDockPanel;
     private detailPanel: Panel;
     private masterPanel: SplitPanel;
-    private element: HTMLElement;
+    element: HTMLElement;
     menu_elem: HTMLElement;
     private _tables: ObservableMap<string, psp.Table | Promise<psp.Table>>;
     private listeners: WeakMap<PerspectiveViewerWidget, () => void>;
@@ -91,6 +91,8 @@ export class PerspectiveWorkspace extends SplitPanel {
         this.commands = createCommands(this, this.indicator);
         this.menu_elem = document.createElement("perspective-workspace-menu");
         this.menu_elem.attachShadow({ mode: "open" });
+        this.menu_elem.shadowRoot!.innerHTML = `<style>${injectedStyles}</style>`;
+
         this.element.shadowRoot!.insertBefore(
             this.menu_elem,
             this.element.shadowRoot!.lastElementChild!
@@ -789,16 +791,13 @@ export class PerspectiveWorkspace extends SplitPanel {
 
     showContextMenu(widget: PerspectiveViewerWidget | null, event: MouseEvent) {
         if (!event.shiftKey) {
-            this.menu_elem.shadowRoot!.innerHTML = `<style>${injectedStyles}</style>`;
-
             const menu = this.createContextMenu(widget);
             menu.init_overlay?.();
             const rect = this.element.getBoundingClientRect();
             menu.open(event.clientX - rect.x, event.clientY - rect.y, {
                 host: this.menu_elem.shadowRoot as unknown as HTMLElement,
             });
 
-            // this.menu_elem = menu_elem;
             event.preventDefault();
             event.stopPropagation();
         }
@@ -828,11 +827,12 @@ export class PerspectiveWorkspace extends SplitPanel {
     }
 
     addViewer(config: ViewerConfigUpdateExt, is_global_filter?: boolean) {
-        const widget = this._createWidgetAndNode({ config });
         if (this.dockpanel.mode === "single-document") {
+            const _task = this._maximizedWidget!.viewer.toggleConfig(false);
             this._unmaximize();
         }
 
+        const widget = this._createWidgetAndNode({ config });
         if (is_global_filter) {
             if (!this.masterPanel.isAttached) {
                 this.setupMasterPanel(DEFAULT_WORKSPACE_SIZE);
@@ -966,6 +966,7 @@ export class PerspectiveWorkspace extends SplitPanel {
         const updated = async (event: CustomEvent) => {
             this.workspaceUpdated();
             widget.title.label = event.detail.title;
+            widget._is_pivoted = event.detail.group_by?.length > 0;
         };
 
         widget.node.addEventListener("contextmenu", contextMenu);

rust/perspective-viewer/src/rust/custom_elements/viewer.rs

@@ -664,20 +664,22 @@ impl PerspectiveViewerElement {
                 .collect();
 
             let theme_name = presentation.get_selected_theme_name().await;
-            presentation.reset_available_themes(themes).await;
+            let mut changed = presentation.reset_available_themes(themes).await;
             let reset_theme = presentation
                 .get_available_themes()
                 .await?
                 .iter()
                 .find(|y| theme_name.as_ref() == Some(y))
                 .cloned();
 
-            presentation.set_theme_name(reset_theme.as_deref()).await?;
-            if let Some(view) = session.get_view() {
-                renderer.restyle_all(&view).await
-            } else {
-                Ok(JsValue::UNDEFINED)
+            changed = presentation.set_theme_name(reset_theme.as_deref()).await? || changed;
+            if changed {
+                if let Some(view) = session.get_view() {
+                    return renderer.restyle_all(&view).await;
+                }
             }
+
+            Ok(JsValue::UNDEFINED)
         })
     }
 

rust/perspective-viewer/src/rust/presentation.rs

@@ -11,7 +11,7 @@
 // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
 
 use std::cell::RefCell;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::ops::Deref;
 use std::rc::Rc;
 
@@ -186,9 +186,20 @@ impl Presentation {
 
     /// Reset the state.  `styleSheets` will be re-parsed next time
     /// `get_themes()` is called if the `themes` argument is `None`.
-    pub async fn reset_available_themes(&self, themes: Option<Vec<String>>) {
+    ///
+    /// # Returns
+    /// A `bool` indicating whether the internal state changed.
+    pub async fn reset_available_themes(&self, themes: Option<Vec<String>>) -> bool {
+        fn as_set(x: &Option<Vec<String>>) -> HashSet<&'_ String> {
+            x.as_ref()
+                .map(|x| x.iter().collect::<HashSet<_>>())
+                .unwrap_or_default()
+        }
+
         let mut mutex = self.0.theme_data.lock().await;
+        let changed = as_set(&mutex.themes) != as_set(&themes);
         mutex.themes = themes;
+        changed
     }
 
     pub async fn get_selected_theme_config(&self) -> ApiResult<(Vec<String>, Option<usize>)> {
@@ -218,8 +229,17 @@ impl Presentation {
     }
 
     /// Set the theme by name, or `None` for the default theme.
-    pub async fn set_theme_name(&self, theme: Option<&str>) -> ApiResult<()> {
-        let (themes, _) = self.get_selected_theme_config().await?;
+    ///
+    /// # Returns
+    /// A `bool` indicating whether the internal state changed.
+    pub async fn set_theme_name(&self, theme: Option<&str>) -> ApiResult<bool> {
+        let (themes, selected) = self.get_selected_theme_config().await?;
+        if let Some(x) = selected {
+            if themes.get(x).map(|x| x.as_str()) == theme {
+                return Ok(false);
+            }
+        }
+
         let index = if let Some(theme) = theme {
             self.set_theme_attribute(Some(theme))?;
             themes.iter().position(|x| x == theme)
@@ -232,7 +252,7 @@ impl Presentation {
         };
 
         self.theme_config_updated.emit((themes, index));
-        Ok(())
+        Ok(true)
     }
 
     /// Returns an owned copy of the curent column configuration map.