Merge pull request #1294 from qdraw/feature/202312_code_smells

move from div to button and code smells fix

starsky/package.json

@@ -3,6 +3,7 @@
   "scripts": {
     "preinstall": "cd starsky && cd clientapp && npm ci",
     "start" : "cd starsky && cd clientapp && npm run start",
+    "build" : "cd starsky && cd clientapp && npm run build",
     "test" : "cd starsky && cd clientapp && npm run test",
     "test:ci" : "cd starsky && cd clientapp && npm run test:ci",
     "storybook" : "cd starsky && cd clientapp && npm run storybook",

starsky/starsky.feature.health/UpdateCheck/Services/CheckForUpdates.cs

@@ -1,6 +1,7 @@
 #nullable enable
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Text.Json;
@@ -39,6 +40,7 @@ public CheckForUpdates(IHttpClientHelper httpClientHelper, AppSettings? appSetti
 		/// </summary>
 		/// <param name="currentVersion">defaults to _appSettings</param>
 		/// <returns></returns>
+		[SuppressMessage("Usage", "S2589:cache & appSettings null")]
 		public async Task<KeyValuePair<UpdateStatus, string>> IsUpdateNeeded(string currentVersion = "")
 		{
 			if (_appSettings == null || _appSettings.CheckForUpdates == false ) 

starsky/starsky.foundation.realtime/Services/HeartBeatService.cs

@@ -30,7 +30,12 @@ public Task StartAsync(CancellationToken cancellationToken)
 			_cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
 
 			_heartbeatTask = HeartbeatAsync(_cancellationTokenSource.Token);
-
+
+			if ( _heartbeatTask.IsCompleted )
+			{
+				_cancellationTokenSource.Dispose();
+			}
+
 			return _heartbeatTask.IsCompleted ? _heartbeatTask : Task.CompletedTask;
 		}
 

starsky/starsky.foundation.sync/WatcherServices/BufferingFileSystemWatcher.cs

@@ -255,6 +255,7 @@ internal void BufferEvent(object _, FileSystemEventArgs e)
         internal void StopRaisingBufferedEvents(object _ = null, EventArgs __ = null)
         {
             _cancellationTokenSource?.Cancel();
+            _cancellationTokenSource?.Dispose();
             _fileSystemEventBuffer = new BlockingCollection<FileSystemEventArgs>(EventQueueCapacity);
         }
 
@@ -421,6 +422,7 @@ protected override void Dispose(bool disposing)
             if (disposing)
             {
                 _cancellationTokenSource?.Cancel();
+                _cancellationTokenSource?.Dispose();
                 _containedFsw?.Dispose();
             }
             base.Dispose(disposing);

starsky/starsky.foundation.worker/Helpers/ProcessTaskQueue.cs

@@ -1,4 +1,3 @@
-#nullable enable
 using System;
 using System.Collections.Generic;
 using System.Linq;

starsky/starsky.sln.DotSettings

@@ -1,4 +1,7 @@
 <wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
+	<s:Boolean x:Key="/Default/CodeEditing/ContextActionTable/DisabledContextActions/=JetBrains_002EReSharper_002EIntentions_002EContextActions_002ECommentSelectionAction/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/CodeEditing/ContextActionTable/DisabledContextActions/=JetBrains_002EReSharper_002EIntentions_002ECSharp_002EContextActions_002EComments_002EDeleteCommentAction/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/CodeEditing/ContextActionTable/DisabledContextActions/=JetBrains_002EReSharper_002EIntentions_002ERazor_002EContextActions_002EComments_002EDeleteCommentAction/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=accountmanagement/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=databasetelemetry/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=exiftool/@EntryIndexedValue">True</s:Boolean>

starsky/starsky/clientapp/.storybook/main.ts

@@ -13,6 +13,9 @@ const config: StorybookConfig = {
   },
   docs: {
     autodocs: "tag"
-  }
+  },
+  core: {
+    disableTelemetry: true, // 👈 Disables telemetry
+  },
 };
 export default config;

starsky/starsky/clientapp/.vscode/settings.json

@@ -3,6 +3,6 @@
   "editor.formatOnSave": true,
   "eslint.alwaysShowStatus": true,
   "editor.codeActionsOnSave": {
-    "source.fixAll.eslint": true
+    "source.fixAll.eslint": "explicit"
   }
 }

starsky/starsky/clientapp/clientapp.code-workspace

@@ -6,10 +6,10 @@
 	],
 	"settings": {
 		"editor.codeActionsOnSave": {
-			"source.organizeImports": true,
-			"source.fixAll": true,
-			"source.fixAll.eslint": true
-		},
+      "source.organizeImports": "explicit",
+      "source.fixAll": "explicit",
+      "source.fixAll.eslint": "explicit"
+    },
 		"editor.formatOnPaste": true,
 		"editor.tabSize": 2,
 		"editor.detectIndentation": false,

starsky/starsky/clientapp/package.json

@@ -17,7 +17,7 @@
     "test:ci:debug": "jest --ci --coverage",
     "preview": "vite preview",
     "tsc": "tsc",
-    "storybook": "storybook dev -p 6006",
+    "storybook": "storybook dev -p 6006 --disable-telemetry",
     "build-storybook": "storybook build",
     "update:install": "npx  --yes npm-check-updates -u && npm install"
   },

starsky/starsky/clientapp/src/components/atoms/menu-option-modal/menu-option-modal.spec.tsx

@@ -0,0 +1,104 @@
+import { fireEvent, render, screen } from "@testing-library/react";
+import MenuOptionModal from "./menu-option-modal.tsx";
+
+describe("MenuOption component", () => {
+  it("expect content", () => {
+    const setMock = jest.fn();
+    const setEnableMoreMenuMock = jest.fn();
+    render(
+      <MenuOptionModal
+        localization={{ nl: "Content", en: "Content" }}
+        isSet={false}
+        set={setMock}
+        testName="test"
+        isReadOnly={true}
+        setEnableMoreMenu={setEnableMoreMenuMock}
+      />
+    );
+
+    expect(screen.getByTestId("test")).toBeTruthy();
+    expect(screen.getByTestId("test").innerHTML).toBe("Content");
+  });
+
+  it("expect child no localisation field", () => {
+    const setMock = jest.fn();
+    const setEnableMoreMenuMock = jest.fn();
+    render(
+      <MenuOptionModal
+        isSet={false}
+        set={setMock}
+        testName="test"
+        isReadOnly={true}
+        setEnableMoreMenu={setEnableMoreMenuMock}
+      >
+        <div>Content</div>
+      </MenuOptionModal>
+    );
+
+    expect(screen.getByTestId("test")).toBeTruthy();
+    expect(screen.getByTestId("test").innerHTML).toBe("<div>Content</div>");
+  });
+
+  it("should not trigger setEnableMoreMenu when isReadOnly is true", () => {
+    const setMock = jest.fn();
+    const setEnableMoreMenuMock = jest.fn();
+    render(
+      <MenuOptionModal
+        localization={{ nl: "Nederlands", en: "English" }}
+        isSet={false}
+        set={setMock}
+        testName="test"
+        isReadOnly={true}
+        setEnableMoreMenu={setEnableMoreMenuMock}
+      />
+    );
+
+    fireEvent.click(screen.getByTestId("test"));
+
+    expect(setMock).toHaveBeenCalledTimes(0);
+    expect(setEnableMoreMenuMock).not.toHaveBeenCalled();
+  });
+
+  it("should trigger setEnableMoreMenu when isReadOnly is false", () => {
+    const setMock = jest.fn();
+    const setEnableMoreMenuMock = jest.fn();
+    render(
+      <MenuOptionModal
+        localization={{ nl: "Nederlands", en: "English" }}
+        isSet={false}
+        set={setMock}
+        testName="test"
+        isReadOnly={false}
+        setEnableMoreMenu={setEnableMoreMenuMock}
+      />
+    );
+    expect(screen.getByTestId("test")).toBeTruthy();
+
+    fireEvent.click(screen.getByTestId("test"));
+
+    expect(setMock).toHaveBeenCalledTimes(1);
+    expect(setEnableMoreMenuMock).toHaveBeenCalled();
+  });
+
+  it("missing localisation field", () => {
+    const setMock = jest.fn();
+    const setEnableMoreMenuMock = jest.fn();
+    render(
+      <MenuOptionModal
+        isSet={false}
+        set={setMock}
+        testName="test"
+        isReadOnly={false}
+        setEnableMoreMenu={setEnableMoreMenuMock}
+      />
+    );
+
+    expect(screen.getByTestId("test")).toBeTruthy();
+    expect(screen.getByTestId("test").innerHTML).toBe("");
+
+    fireEvent.click(screen.getByTestId("test"));
+
+    expect(setMock).toHaveBeenCalledTimes(1);
+    expect(setEnableMoreMenuMock).toHaveBeenCalled();
+  });
+});

starsky/starsky/clientapp/src/components/atoms/menu-option-modal/menu-option-modal.stories.tsx

@@ -0,0 +1,28 @@
+import MenuOptionModal from "./menu-option-modal.tsx";
+
+export default {
+  title: "components/atoms/menu-option-modal"
+};
+
+export const Default = () => {
+  return (
+    <div className="menu-context">
+      <ul className="menu-options">
+        <MenuOptionModal
+          localization={{ nl: "Nederlands", en: "English" }}
+          isSet={false}
+          set={() => {}}
+          testName="test"
+          isReadOnly={false}
+          setEnableMoreMenu={(value) => {
+            alert(value);
+          }}
+        />
+      </ul>
+    </div>
+  );
+};
+
+Default.story = {
+  name: "default"
+};

starsky/starsky/clientapp/src/components/atoms/menu-option-modal/menu-option-modal.tsx

@@ -0,0 +1,62 @@
+import React, { memo } from "react";
+import useGlobalSettings from "../../../hooks/use-global-settings";
+import { Language } from "../../../shared/language";
+
+interface IMenuOptionModalProps {
+  isReadOnly: boolean;
+  testName: string;
+  isSet: boolean;
+  set: React.Dispatch<React.SetStateAction<boolean>>;
+  localization?: { nl: string; en: string };
+  setEnableMoreMenu?: React.Dispatch<React.SetStateAction<boolean>>;
+  children?: React.ReactNode;
+}
+
+const MenuOptionModal: React.FunctionComponent<IMenuOptionModalProps> = memo(
+  ({
+    localization,
+    isSet,
+    set,
+    testName,
+    isReadOnly = true,
+    setEnableMoreMenu = undefined,
+    children = undefined
+  }) => {
+    const settings = useGlobalSettings();
+    const language = new Language(settings.language);
+
+    const Message = !localization ? "" : language.key(localization);
+
+    function onClickHandler() {
+      if (isReadOnly) {
+        return;
+      }
+      // close menu
+      if (setEnableMoreMenu) {
+        setEnableMoreMenu(false);
+      }
+      set(!isSet);
+    }
+
+    return (
+      <>
+        {
+          <li className={!isReadOnly ? "menu-option" : "menu-option disabled"}>
+            <button
+              data-test={testName}
+              onClick={onClickHandler}
+              onKeyDown={(event) => {
+                event.key === "Enter" && onClickHandler();
+              }}
+            >
+              {Message}
+              {children}
+            </button>
+          </li>
+        }
+      </>
+    );
+  }
+);
+
+export default MenuOptionModal;