Merge pull request #1271 from qdraw/feature/202311_code_smells

fix some code smells

.github/workflows.md

@@ -0,0 +1,6 @@
+
+The following workflows are allowed on: https://github.com/qdraw/starsky/settings/actions
+
+```
+ljharb/rebase@master@,peter-evans/create-pull-request@*,EndBug/add-and-commit@*,everlytic/branch-merge*,docker/login-action@*,docker/metadata-action@*,docker/build-push-action@*,codecov/codecov-action@*,chizkiyahu/delete-untagged-ghcr-action@*,cypress-io/github-action@*,softprops/action-gh-release@*
+```

starsky/Dockerfile

@@ -60,7 +60,7 @@ RUN \
 
 RUN \
     if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \
-        echo $TARGETPLATFORM ; \
+        echo "$TARGETPLATFORM" ; \
         dotnet publish -c release -o out --runtime linux-x64 --self-contained false --no-restore ; \
     elif [ "$TARGETPLATFORM" = "linux/arm64" ]; then \
         dotnet publish -c release -o out --runtime linux-arm64 --self-contained false --no-restore ; \

starsky/starsky.foundation.http/Streaming/FileStreamingHelper.cs

@@ -53,6 +53,7 @@ public static async Task<List<string>> StreamFile(this HttpRequest request,
         }
 
         [SuppressMessage("Usage", "S125:Remove this commented out code")]
+        [SuppressMessage("Usage", "S2589:contentDisposition null")]
         public static async Task<List<string>> StreamFile(string? contentType, Stream requestBody, AppSettings appSettings, 
 	        ISelectorStorage selectorStorage, string? headerFileName = null)
         {

starsky/starsky.foundation.webtelemetry/Services/ApplicationInsightsJsHelper.cs

@@ -93,7 +93,7 @@ public string ScriptPlain
 
 		internal string? GetCurrentUserId()
 		{
-			if (_httpContext == null || _httpContext?.HttpContext?.User.Identity?.IsAuthenticated == false)
+			if (_httpContext?.HttpContext?.User.Identity?.IsAuthenticated == false)
 			{
 				return string.Empty;
 			}

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

@@ -5,7 +5,6 @@ const config: StorybookConfig = {
   addons: [
     "@storybook/addon-links",
     "@storybook/addon-essentials",
-    "@storybook/addon-onboarding",
     "@storybook/addon-interactions"
   ],
   framework: {

starsky/starsky/clientapp/package.json

@@ -168,7 +168,7 @@
       "@typescript-eslint/no-explicit-any": "warn",
       "@typescript-eslint/ban-types": "warn",
       "no-case-declarations": "warn",
-      "react/display-name": "warn",
+      "react/display-name": "off",
       "react/prop-types": "warn",
       "@typescript-eslint/no-loss-of-precision": "warn",
       "react/react-in-jsx-scope": "off"

starsky/starsky/clientapp/src/components/atoms/drop-area/drop-area.tsx

@@ -34,7 +34,7 @@ const containsFiles = (event: DragEvent) => {
  * @param props Endpoints, settings to enable drag 'n drop, add extra classes
  */
 const DropArea: React.FunctionComponent<IDropAreaProps> = (props) => {
-  const [dragActive, setDrag] = useState(false);
+  const [dragActive, setDragActive] = useState(false);
   const [dragTarget, setDragTarget] = useState(
     document.createElement("span") as Element
   );
@@ -65,7 +65,7 @@ const DropArea: React.FunctionComponent<IDropAreaProps> = (props) => {
    */
   const onDrop = (event: DragEvent) => {
     event.preventDefault();
-    setDrag(false);
+    setDragActive(false);
 
     if (!event.dataTransfer) return;
 
@@ -89,7 +89,7 @@ const DropArea: React.FunctionComponent<IDropAreaProps> = (props) => {
   const onDragEnter = (event: DragEvent) => {
     event.preventDefault();
     if (!event.target || !containsFiles(event)) return;
-    setDrag(true);
+    setDragActive(true);
     setDragTarget(event.target as Element);
     setDropEffect(event);
   };
@@ -103,7 +103,7 @@ const DropArea: React.FunctionComponent<IDropAreaProps> = (props) => {
     event.preventDefault();
     if (!containsFiles(event) || (event.target as Element) !== dragTarget)
       return;
-    setDrag(false);
+    setDragActive(false);
   };
 
   /**
@@ -113,7 +113,7 @@ const DropArea: React.FunctionComponent<IDropAreaProps> = (props) => {
   const onDragOver = (event: DragEvent) => {
     event.preventDefault();
     if (!containsFiles(event)) return;
-    setDrag(true);
+    setDragActive(true);
     setDropEffect(event);
   };
 

starsky/starsky/clientapp/src/components/atoms/file-hash-image/file-hash-image.spec.tsx

@@ -52,10 +52,7 @@ describe("FileHashImage", () => {
 
     // need to await here
     const component = await render(
-      <FileHashImage
-        fileHash="hash"
-        orientation={Orientation.Horizontal}
-      />
+      <FileHashImage fileHash="hash" orientation={Orientation.Horizontal} />
     );
     expect(detectRotationSpy).toBeCalled();
 
@@ -94,10 +91,7 @@ describe("FileHashImage", () => {
       .mockImplementationOnce(() => mockGetIConnectionDefault);
 
     const component = render(
-      <FileHashImage
-        fileHash="hash"
-        orientation={Orientation.Horizontal}
-      />
+      <FileHashImage fileHash="hash" orientation={Orientation.Horizontal} />
     );
 
     // need to await here
@@ -125,10 +119,7 @@ describe("FileHashImage", () => {
       .mockImplementationOnce(() => mockGetIConnectionDefault);
 
     const component = render(
-      <FileHashImage
-        fileHash="hash"
-        orientation={Orientation.Horizontal}
-      />
+      <FileHashImage fileHash="hash" orientation={Orientation.Horizontal} />
     );
 
     // need to await here

starsky/starsky/clientapp/src/components/atoms/file-hash-image/pan-and-zoom-image.tsx

@@ -31,7 +31,7 @@ export type PositionObject = {
  * @param param0:  IPanAndZoomImage
  */
 const PanAndZoomImage = ({ src, id, ...props }: IPanAndZoomImage) => {
-  const [isPanning, setPanning] = useState(false);
+  const [panning, setPanning] = useState(false);
   const [image, setImage] = useState({ width: 0, height: 0 } as ImageObject);
 
   const defaultPosition = {
@@ -66,12 +66,12 @@ const PanAndZoomImage = ({ src, id, ...props }: IPanAndZoomImage) => {
     };
     // for performance reasons the classes is kept in a function
     const mouseMove = (event: MouseEvent) => {
-      new OnMoveMouseTouchAction(isPanning, setPosition, position).mousemove(
+      new OnMoveMouseTouchAction(panning, setPosition, position).mousemove(
         event
       );
     };
     const touchMove = (event: TouchEvent) => {
-      new OnMoveMouseTouchAction(isPanning, setPosition, position).touchMove(
+      new OnMoveMouseTouchAction(panning, setPosition, position).touchMove(
         event
       );
     };
@@ -121,7 +121,7 @@ const PanAndZoomImage = ({ src, id, ...props }: IPanAndZoomImage) => {
 
   let className = "pan-zoom-image-container";
   if (position.z !== 1) {
-    className = !isPanning
+    className = !panning
       ? "pan-zoom-image-container grab"
       : "pan-zoom-image-container is-panning";
   }

starsky/starsky/clientapp/src/components/atoms/list-image/list-image.tsx

@@ -22,9 +22,7 @@ const ListImage: React.FunctionComponent<IListImageProps> = memo((props) => {
 
   const [src, setSrc] = useState(props.fileHash);
 
-  const [alwaysLoadImage] = useState(
-    localStorage.getItem("alwaysLoadImage") === "true"
-  );
+  const alwaysLoadImage = localStorage.getItem("alwaysLoadImage") === "true";
 
   // Reset Loading after changing page
   const [isLoading, setIsLoading] = useState(true);
@@ -45,7 +43,8 @@ const ListImage: React.FunctionComponent<IListImageProps> = memo((props) => {
 
   // to stop loading images after a url change
   const history = useLocation();
-  const [historyLocation] = useState(history.location.search);
+  const historyLocation = history.location.search;
+
   useEffect(() => {
     // use ?f only to support details
     // need to refresh

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

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

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

@@ -11,6 +11,7 @@ interface IMenuOptionProps {
   setEnableMoreMenu?: React.Dispatch<React.SetStateAction<boolean>>;
 }
 
+// eslint-disable-next-line react/display-name
 const MenuOption: React.FunctionComponent<IMenuOptionProps> = memo(
   ({
     localization,
@@ -24,21 +25,27 @@ const MenuOption: React.FunctionComponent<IMenuOptionProps> = memo(
     const language = new Language(settings.language);
     const Message = language.key(localization);
 
+    function onClickHandler() {
+      if (isReadOnly) {
+        return;
+      }
+      // close menu
+      if (setEnableMoreMenu) {
+        setEnableMoreMenu(false);
+      }
+      set(!isSet);
+    }
+
     return (
       <>
         {
           <li
             tabIndex={0}
             data-test={testName}
             className={!isReadOnly ? "menu-option" : "menu-option disabled"}
-            onClick={() => {
-              if (!isReadOnly) {
-                // close menu
-                if (setEnableMoreMenu) {
-                  setEnableMoreMenu(false);
-                }
-                set(!isSet);
-              }
+            onClick={onClickHandler}
+            onKeyDown={(event) => {
+              event.key === "Enter" && onClickHandler();
             }}
           >
             {Message}

starsky/starsky/clientapp/src/components/atoms/modal/modal.spec.tsx

@@ -1,4 +1,9 @@
-import { render, RenderResult, screen } from "@testing-library/react";
+import {
+  fireEvent,
+  render,
+  RenderResult,
+  screen
+} from "@testing-library/react";
 import Modal from "./modal";
 
 describe("Modal", () => {
@@ -26,14 +31,36 @@ describe("Modal", () => {
       };
     }
 
-    it("modal-exit-button", () => {
+    it("modal-exit-button click", () => {
       const { handleExit, element } = renderModal();
 
       screen.queryAllByTestId("modal-exit-button")[0].click();
       expect(handleExit).toBeCalled();
       element.unmount();
     });
 
+    it("modal-exit-button keyDown tab ignores", () => {
+      const { handleExit, element } = renderModal();
+
+      const menuOption = screen.queryAllByTestId("modal-exit-button")[0];
+
+      fireEvent.keyDown(menuOption, { key: "Tab" });
+
+      expect(handleExit).toBeCalledTimes(0);
+      element.unmount();
+    });
+
+    it("modal-exit-button keyDown enter", () => {
+      const { handleExit, element } = renderModal();
+
+      const menuOption = screen.queryAllByTestId("modal-exit-button")[0];
+
+      fireEvent.keyDown(menuOption, { key: "Enter" });
+
+      expect(handleExit).toBeCalledTimes(1);
+      element.unmount();
+    });
+
     it("modal-bg", () => {
       const { handleExit, element } = renderModal();
 

starsky/starsky/clientapp/src/components/atoms/modal/modal.tsx

@@ -42,13 +42,13 @@ export default function Modal({
   const language = new Language(settings.language);
   const MessageCloseDialog = language.text("Sluiten", "Close");
 
-  const [hasUpdated, forceUpdate] = useState(false);
+  const [forceUpdate, setForceUpdate] = useState(false);
 
   const exitButton = useRef<HTMLButtonElement>(null);
   const modal = useRef<HTMLDivElement | null>(null);
 
   useEffect(() => {
-    return modalInsertPortalDiv(modal, hasUpdated, forceUpdate, id);
+    return modalInsertPortalDiv(modal, forceUpdate, setForceUpdate, id);
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
@@ -68,6 +68,9 @@ export default function Modal({
     return ReactDOM.createPortal(
       <div
         onClick={(event) => ifModalOpenHandleExit(event, handleExit)}
+        onKeyDown={(event) => {
+          event.key === "Enter" && handleExit();
+        }}
         data-test={dataTest}
         className={`modal-bg ${
           isOpen ? ` ${ModalOpenClassName} ` + className : ""

starsky/starsky/clientapp/src/components/atoms/more-menu/more-menu.stories.tsx

@@ -5,7 +5,15 @@ export default {
 };
 
 export const Default = () => {
-  return <MoreMenu setEnableMoreMenu={() => {}}>test</MoreMenu>;
+  return (
+    <MoreMenu
+      setEnableMoreMenu={() => {
+        alert("test");
+      }}
+    >
+      test
+    </MoreMenu>
+  );
 };
 
 Default.story = {

starsky/starsky/clientapp/src/components/atoms/more-menu/more-menu.tsx

@@ -47,6 +47,9 @@ const MoreMenu: React.FunctionComponent<MoreMenuPropTypes> = ({
       <div
         onChange={offMoreMenu}
         onClick={() => setEnableMoreMenu(false)}
+        onKeyDown={(event) => {
+          event.key === "Enter" && setEnableMoreMenu(false);
+        }}
         data-test="menu-context"
         className={
           enableMoreMenu ? "menu-context" : "menu-context menu-context--hide"

starsky/starsky/clientapp/src/components/molecules/archive-sidebar/archive-sidebar-label-edit-add-overwrite.tsx

@@ -76,7 +76,7 @@ const ArchiveSidebarLabelEditAddOverwrite: React.FunctionComponent = () => {
   } as ISidebarUpdate);
 
   // Add/Hide disabled state
-  const [isInputEnabled, setInputEnabled] = useState(false);
+  const [inputEnabled, setInputEnabled] = useState(false);
 
   // preloading icon
   const [isLoading, setIsLoading] = useState(false);
@@ -217,7 +217,7 @@ const ArchiveSidebarLabelEditAddOverwrite: React.FunctionComponent = () => {
         contentEditable={!state.isReadOnly && select.length !== 0}
       ></FormControl>
 
-      {isInputEnabled && select.length !== 0 ? (
+      {inputEnabled && select.length !== 0 ? (
         <button
           className="btn btn--info"
           data-test="overwrite"
@@ -230,7 +230,7 @@ const ArchiveSidebarLabelEditAddOverwrite: React.FunctionComponent = () => {
           {MessageOverwriteName}
         </button>
       )}
-      {isInputEnabled && select.length !== 0 ? (
+      {inputEnabled && select.length !== 0 ? (
         <button
           data-test="add"
           className="btn btn--default"