Fixes (#4730)

cognitedata · Aug 29, 2024 · 5afcb69 · 5afcb69
1 parent 5be4f0e
commit 5afcb69
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 22 deletions.
diff --git a/react-components/src/architecture/base/commands/BaseFilterCommand.ts b/react-components/src/architecture/base/commands/BaseFilterCommand.ts
@@ -6,6 +6,7 @@ import { RenderTargetCommand } from './RenderTargetCommand';
 import { type TranslateDelegate } from '../utilities/TranslateKey';
 import { type Color } from 'three';
 import { type BaseCommand } from './BaseCommand';
+import { CommandsUpdater } from '../reactUpdaters/CommandsUpdater';
 
 export abstract class BaseFilterCommand extends RenderTargetCommand {
   // ==================================================
@@ -74,15 +75,17 @@ export abstract class BaseFilterCommand extends RenderTargetCommand {
    * Toggles the checked state of all child filter items.
    * Override this method to optimize the logic.
    * If there are no child items, this method does nothing.
+   * @returns A boolean value indicating whether anything is changed
    */
-  public toggleAllChecked(): void {
+  protected toggleAllCheckedCore(): boolean {
     if (this._children === undefined || this._children.length === 0) {
-      return;
+      return false;
     }
     const isAllChecked = this.isAllChecked;
     for (const child of this._children) {
       child.setChecked(!isAllChecked);
     }
+    return true;
   }
 
   // ==================================================
@@ -107,6 +110,12 @@ export abstract class BaseFilterCommand extends RenderTargetCommand {
     return counter.toString() + ' ' + BaseFilterCommand.getSelectedString(translate);
   }
 
+  public toggleAllChecked(): void {
+    if (this.toggleAllCheckedCore()) {
+      CommandsUpdater.update(this._renderTarget);
+    }
+  }
+
   // ==================================================
   // STATIC METHODS
   // ==================================================
@@ -125,6 +134,19 @@ export abstract class BaseFilterCommand extends RenderTargetCommand {
 }
 
 export abstract class BaseFilterItemCommand extends RenderTargetCommand {
-  public abstract get color(): Color | undefined;
-  public abstract setChecked(value: boolean): void;
+  // ==================================================
+  // VIRTUAL METHODS (To be overridden)
+  // ==================================================
+
+  public get color(): Color | undefined {
+    return undefined;
+  }
+
+  protected abstract setCheckedCore(value: boolean): boolean;
+
+  public setChecked(value: boolean): void {
+    if (this.setCheckedCore(value)) {
+      CommandsUpdater.update(this._renderTarget);
+    }
+  }
 }
diff --git a/react-components/src/architecture/base/commands/mocks/MockFilterCommand.ts b/react-components/src/architecture/base/commands/mocks/MockFilterCommand.ts
@@ -5,7 +5,6 @@
 import { Color } from 'three';
 import { BaseFilterCommand, BaseFilterItemCommand } from '../BaseFilterCommand';
 import { type TranslateKey } from '../../utilities/TranslateKey';
-import { CommandsUpdater } from '../../reactUpdaters/CommandsUpdater';
 
 export class MockFilterCommand extends BaseFilterCommand {
   // ==================================================
@@ -98,11 +97,11 @@ class FilterItemCommand extends BaseFilterItemCommand {
     return this._color;
   }
 
-  public override setChecked(value: boolean): void {
+  protected override setCheckedCore(value: boolean): boolean {
     if (this._use === value) {
-      return;
+      return false;
     }
     this._use = value;
-    CommandsUpdater.update(this._renderTarget);
+    return true;
   }
 }
diff --git a/react-components/src/architecture/base/concreteCommands/PointCloudFilterCommand.ts b/react-components/src/architecture/base/concreteCommands/PointCloudFilterCommand.ts
@@ -6,7 +6,6 @@ import { type CognitePointCloudModel, type WellKnownAsprsPointClassCodes } from
 import { type TranslateKey } from '../utilities/TranslateKey';
 import { type Color } from 'three';
 import { BaseFilterCommand, BaseFilterItemCommand } from '../commands/BaseFilterCommand';
-import { CommandsUpdater } from '../reactUpdaters/CommandsUpdater';
 import { type RevealRenderTarget } from '../renderTarget/RevealRenderTarget';
 
 export class PointCloudFilterCommand extends BaseFilterCommand {
@@ -71,19 +70,20 @@ export class PointCloudFilterCommand extends BaseFilterCommand {
     return isAllClassesVisible(pointCloud);
   }
 
-  public override toggleAllChecked(): void {
+  protected override toggleAllCheckedCore(): boolean {
     const pointCloud = this.getPointCloud();
     if (pointCloud === undefined) {
-      return;
+      return false;
     }
     const isAllChecked = isAllClassesVisible(pointCloud);
     const classes = pointCloud.getClasses();
     if (classes === undefined || classes.length === 0) {
-      return;
+      return false;
     }
     for (const c of classes) {
       pointCloud.setClassVisible(c.code, !isAllChecked);
     }
+    return true;
   }
 
   // ==================================================
@@ -151,13 +151,16 @@ export class FilterItemCommand extends BaseFilterItemCommand {
     return this._pointClass.color;
   }
 
-  public override setChecked(value: boolean): void {
+  protected override setCheckedCore(value: boolean): boolean {
     const pointCloud = this.getPointCloud();
     if (pointCloud === undefined) {
-      return;
+      return false;
+    }
+    if (pointCloud.isClassVisible(this._pointClass.code) === value) {
+      return false;
     }
     pointCloud.setClassVisible(this._pointClass.code, value);
-    CommandsUpdater.update(this._renderTarget);
+    return true;
   }
 
   // ==================================================

diff --git a/react-components/src/components/Architecture/FilterButton.tsx b/react-components/src/components/Architecture/FilterButton.tsx
@@ -27,6 +27,7 @@ import { useClickOutside } from './useClickOutside';
 import styled from 'styled-components';
 import { BaseFilterCommand } from '../../architecture/base/commands/BaseFilterCommand';
 import { FilterItem } from './FilterItem';
+import { OPTION_MIN_WIDTH, DEFAULT_PADDING } from './constants';
 
 export const FilterButton = ({
   inputCommand,
@@ -143,8 +144,8 @@ export const FilterButton = ({
           iconPlacement="right"
           aria-label={command.getLabel(t)}
           style={{
-            minWidth: usedInSettings ? '100px' : undefined,
-            padding: usedInSettings ? '4px 4px' : undefined
+            minWidth: usedInSettings ? OPTION_MIN_WIDTH : undefined,
+            padding: usedInSettings ? DEFAULT_PADDING : undefined
           }}
           onClick={(event: MouseEvent<HTMLElement>) => {
             setOpen(!isOpen);

diff --git a/react-components/src/components/Architecture/OptionButton.tsx b/react-components/src/components/Architecture/OptionButton.tsx
@@ -26,6 +26,7 @@ import {
 import { LabelWithShortcut } from './LabelWithShortcut';
 import { type TranslateDelegate } from '../../architecture/base/utilities/TranslateKey';
 import { useClickOutside } from './useClickOutside';
+import { DEFAULT_PADDING, OPTION_MIN_WIDTH } from './constants';
 
 export const OptionButton = ({
   inputCommand,
@@ -114,8 +115,8 @@ export const OptionButton = ({
         }>
         <Button
           style={{
-            padding: usedInSettings ? '4px 4px' : '8px 4px',
-            minWidth: usedInSettings ? '100px' : undefined
+            padding: usedInSettings ? DEFAULT_PADDING : '8px 4px',
+            minWidth: usedInSettings ? OPTION_MIN_WIDTH : undefined
           }}
           type={usedInSettings ? 'tertiary' : getButtonType(command)}
           icon={isOpen ? 'ChevronUp' : 'ChevronDown'}

diff --git a/react-components/src/components/Architecture/SettingsButton.tsx b/react-components/src/components/Architecture/SettingsButton.tsx
@@ -39,6 +39,7 @@ import { BaseSliderCommand } from '../../architecture/base/commands/BaseSliderCo
 import { BaseFilterCommand } from '../../architecture/base/commands/BaseFilterCommand';
 import { FilterButton } from './FilterButton';
 import { useClickOutside } from './useClickOutside';
+import { DEFAULT_PADDING } from './constants';
 
 export const SettingsButton = ({
   inputCommand,
@@ -115,7 +116,7 @@ export const SettingsButton = ({
             <Menu
               style={{
                 flexDirection,
-                padding: '4px 4px'
+                padding: DEFAULT_PADDING
               }}>
               {children.map((child, _index): ReactElement | undefined => {
                 return createMenuItem(child, t);
@@ -169,7 +170,7 @@ function createToggle(command: BaseCommand, t: TranslateDelegate): ReactElement
       hasSwitch={true}
       disabled={!command.isEnabled}
       toggled={isChecked}
-      style={{ padding: '4px 4px' }}
+      style={{ padding: DEFAULT_PADDING }}
       onChange={() => {
         command.invoke();
         setChecked(command.isChecked);
@@ -192,7 +193,7 @@ function createButton(command: BaseCommand, t: TranslateDelegate): ReactElement
       toggled={isChecked}
       icon={getIcon(command)}
       iconPlacement="left"
-      style={{ padding: '4px 4px' }}
+      style={{ padding: DEFAULT_PADDING }}
       onClick={() => {
         command.invoke();
         setChecked(command.isChecked);

diff --git a/react-components/src/components/Architecture/constants.tsx b/react-components/src/components/Architecture/constants.tsx
@@ -0,0 +1,6 @@
+/*!
+ * Copyright 2024 Cognite AS
+ */
+
+export const OPTION_MIN_WIDTH = '125px';
+export const DEFAULT_PADDING = '4px 4px';