Fix according to review and remove the mock

cognitedata · Jul 29, 2024 · c15f442 · c15f442
1 parent f44eed2
commit c15f442
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 23 deletions.
diff --git a/react-components/src/architecture/base/concreteCommands/SetPointColorTypeCommand.ts b/react-components/src/architecture/base/concreteCommands/SetPointColorTypeCommand.ts
@@ -36,7 +36,6 @@ export class SetPointColorTypeCommand extends BaseOptionCommand {
   }
 
   public override get isEnabled(): boolean {
-    return true;
     return this.renderTarget.getPointClouds().next().value !== undefined;
   }
 }
@@ -45,7 +44,6 @@ export class SetPointColorTypeCommand extends BaseOptionCommand {
 
 class OptionCommand extends RenderTargetCommand {
   private readonly _value: PointColorType;
-  private static selected = PointColorType.Rgb;
 
   public constructor(value: PointColorType) {
     super();
@@ -60,7 +58,7 @@ class OptionCommand extends RenderTargetCommand {
     // Let the first PointCloud decide the color type
     const pointCloud = this.renderTarget.getPointClouds().next().value;
     if (pointCloud === undefined) {
-      return OptionCommand.selected === this._value;
+      return false;
     }
     return pointCloud.pointColorType === this._value;
   }
@@ -69,7 +67,6 @@ class OptionCommand extends RenderTargetCommand {
     for (const pointCloud of this.renderTarget.getPointClouds()) {
       pointCloud.pointColorType = this._value;
     }
-    OptionCommand.selected = this._value;
     return true;
   }
 }

diff --git a/react-components/src/architecture/base/concreteCommands/SetPointShapeCommand.ts b/react-components/src/architecture/base/concreteCommands/SetPointShapeCommand.ts
@@ -30,7 +30,6 @@ export class SetPointShapeCommand extends BaseOptionCommand {
   }
 
   public override get isEnabled(): boolean {
-    return true;
     return this.renderTarget.getPointClouds().next().value !== undefined;
   }
 }
@@ -39,7 +38,6 @@ export class SetPointShapeCommand extends BaseOptionCommand {
 
 class OptionCommand extends RenderTargetCommand {
   private readonly _value: PointShape;
-  private static selected = PointShape.Circle;
 
   public constructor(value: PointShape) {
     super();
@@ -54,7 +52,7 @@ class OptionCommand extends RenderTargetCommand {
     // Let the first PointCloud decide the color type
     const pointCloud = this.renderTarget.getPointClouds().next().value;
     if (pointCloud === undefined) {
-      return OptionCommand.selected === this._value;
+      return false;
     }
     return pointCloud.pointSizeType === this._value;
   }
@@ -63,7 +61,6 @@ class OptionCommand extends RenderTargetCommand {
     for (const pointCloud of this.renderTarget.getPointClouds()) {
       pointCloud.pointShape = this._value;
     }
-    OptionCommand.selected = this._value;
     return true;
   }
 }

diff --git a/react-components/src/architecture/base/concreteCommands/SetPointSizeCommand.ts b/react-components/src/architecture/base/concreteCommands/SetPointSizeCommand.ts
@@ -5,10 +5,10 @@
 import { type TranslateKey } from '../utilities/TranslateKey';
 import { BaseSliderCommand } from '../commands/BaseSliderCommand';
 
-export const DEFAULT_POINT_SIZE = 2;
-export const MIN_POINT_SIZE = 0.0;
-export const MAX_POINT_SIZE = 4; // Default seems be be 2, but the user probably wants lower values
-export const STEP_POINT_SIZE = 0.1;
+const DEFAULT_POINT_SIZE = 2;
+const MIN_POINT_SIZE = 0.0;
+const MAX_POINT_SIZE = 4; // Default seems be be 2, but the user probably wants lower values
+const STEP_POINT_SIZE = 0.1;
 
 export class SetPointSizeCommand extends BaseSliderCommand {
   // ==================================================
@@ -28,17 +28,13 @@ export class SetPointSizeCommand extends BaseSliderCommand {
   }
 
   public override get isEnabled(): boolean {
-    return true;
     return this.renderTarget.getPointClouds().next().value !== undefined;
   }
 
   public override get value(): number {
     // Let the first PointCloud decide the point size
     const pointCloud = this.renderTarget.getPointClouds().next().value;
-    if (pointCloud === undefined) {
-      return DEFAULT_POINT_SIZE;
-    }
-    return pointCloud.pointSize;
+    return pointCloud?.pointSize ?? DEFAULT_POINT_SIZE;
   }
 
   public override set value(value: number) {

diff --git a/react-components/src/architecture/base/concreteCommands/SettingsCommand.ts b/react-components/src/architecture/base/concreteCommands/SettingsCommand.ts
@@ -39,6 +39,10 @@ export class SettingsCommand extends RenderTargetCommand {
   // ==================================================
 
   public add(command: BaseCommand): void {
+    if (this._commands.find((c) => c.equals(command)) !== undefined) {
+      console.error('Duplicated command given: ' + command.name);
+      return;
+    }
     this._commands.push(command);
   }
 

diff --git a/react-components/src/components/Architecture/SettingsButton.tsx b/react-components/src/components/Architecture/SettingsButton.tsx
@@ -86,17 +86,14 @@ export const SettingsButton = ({
         appendTo={document.body}
         placement="auto-start"
         content={
-          <Menu
+          <StyledMenu
             style={{
-              minWidth: '0px',
-              overflow: 'auto',
-              flexDirection,
-              padding: '4px 4px'
+              flexDirection
             }}>
             {commands.map((command, _index): ReactElement | undefined => {
               return createMenuItem(command, t);
             })}
-          </Menu>
+          </StyledMenu>
         }>
         <Button
           type={getButtonType(command)}
@@ -161,6 +158,7 @@ export function createSlider(command: BaseSliderCommand, t: TranslateDelegate):
   }
   return (
     <SliderDiv>
+      key={command.uniqueId}
       <label>{command.getLabel(t)}</label>
       <StyledSlider
         min={command.min}
@@ -181,12 +179,19 @@ export function createOptionButton(command: BaseOptionCommand, t: TranslateDeleg
   }
   return (
     <OptionDiv>
+      key={command.uniqueId}
       <label>{command.getLabel(t)}</label>
       <OptionButton inputCommand={command} isHorizontal={false} usedInSettings={true} />
     </OptionDiv>
   );
 }
 
+const StyledMenu = styled(Menu)`
+  min-width: '0px',
+  overflow: 'auto',
+  padding: '4px 4px'
+`;
+
 const OptionDiv = styled.div`
   display: flex;
   flex-direction: row;