From f7e250ea85205cafe681ca375c6b80625352a125 Mon Sep 17 00:00:00 2001
From: Iku-turso <mikko.aspiala@gmail.com>
Date: Tue, 18 Oct 2022 16:46:39 +0300
Subject: [PATCH] Make styling less brittle by not relying on static
 HTML-element structures with CSS-rules

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>
---
 .../horizontal-line.module.scss               | 17 ++++
 .../horizontal-line/horizontal-line.tsx       | 16 ++++
 ...application-preference-page.injectable.tsx |  4 +-
 .../kubectl-download-mirror.tsx               |  2 -
 .../kubernetes-preference-page.injectable.tsx |  4 +-
 .../preference-items/preference-tab-root.tsx  |  4 +-
 .../proxy-preference-page.injectable.tsx      |  3 +-
 .../preferences/renderer/preferences.tsx      | 78 ++++++++++---------
 .../components/gutter/gutter.module.scss      |  1 +
 src/renderer/components/gutter/gutter.tsx     |  2 +-
 .../components/layout/setting-layout.scss     | 15 +---
 11 files changed, 89 insertions(+), 57 deletions(-)
 create mode 100644 src/features/preferences/renderer/horizontal-line/horizontal-line.module.scss
 create mode 100644 src/features/preferences/renderer/horizontal-line/horizontal-line.tsx

diff --git a/src/features/preferences/renderer/horizontal-line/horizontal-line.module.scss b/src/features/preferences/renderer/horizontal-line/horizontal-line.module.scss
new file mode 100644
index 0000000000..3d6f09dbfc
--- /dev/null
+++ b/src/features/preferences/renderer/horizontal-line/horizontal-line.module.scss
@@ -0,0 +1,17 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+.HorizontalLine {
+  margin-top: 40px;
+  margin-bottom: 40px;
+
+  height: 1px;
+  border-top: thin solid var(--hrColor);
+}
+
+.Small {
+  margin-top: 20px;
+  margin-bottom: 20px;
+}
diff --git a/src/features/preferences/renderer/horizontal-line/horizontal-line.tsx b/src/features/preferences/renderer/horizontal-line/horizontal-line.tsx
new file mode 100644
index 0000000000..27a5fb3eb6
--- /dev/null
+++ b/src/features/preferences/renderer/horizontal-line/horizontal-line.tsx
@@ -0,0 +1,16 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import React from "react";
+import styles from "./horizontal-line.module.scss";
+import { cssNames } from "../../../../renderer/utils";
+
+export const HorizontalLine = ({ small } = { small: false }) => (
+  <div
+    className={cssNames({
+      [styles.HorizontalLine]: true,
+      [styles.Small]: small,
+    })}
+  />
+);
diff --git a/src/features/preferences/renderer/preference-items/application/application-preference-page.injectable.tsx b/src/features/preferences/renderer/preference-items/application/application-preference-page.injectable.tsx
index 665ce1db62..aa35ff20d6 100644
--- a/src/features/preferences/renderer/preference-items/application/application-preference-page.injectable.tsx
+++ b/src/features/preferences/renderer/preference-items/application/application-preference-page.injectable.tsx
@@ -4,8 +4,8 @@
  */
 import { getInjectable } from "@ogre-tools/injectable";
 import { preferenceItemInjectionToken } from "../preference-item-injection-token";
-import React from "react";
 import { getPreferencePage } from "../../get-preference-page";
+import { HorizontalLine } from "../../horizontal-line/horizontal-line";
 
 const applicationPreferencePageInjectable = getInjectable({
   id: "application-preference-page",
@@ -16,7 +16,7 @@ const applicationPreferencePageInjectable = getInjectable({
     parentId: "application-tab",
     orderNumber: 0,
     Component: getPreferencePage("Application"),
-    childrenSeparator: () => <hr />,
+    childrenSeparator: HorizontalLine,
   }),
 
   injectionToken: preferenceItemInjectionToken,
diff --git a/src/features/preferences/renderer/preference-items/kubernetes/kubectl/kubectl-download-mirror/kubectl-download-mirror.tsx b/src/features/preferences/renderer/preference-items/kubernetes/kubectl/kubectl-download-mirror/kubectl-download-mirror.tsx
index 1630a96882..e880ff3308 100644
--- a/src/features/preferences/renderer/preference-items/kubernetes/kubectl/kubectl-download-mirror/kubectl-download-mirror.tsx
+++ b/src/features/preferences/renderer/preference-items/kubernetes/kubectl/kubectl-download-mirror/kubectl-download-mirror.tsx
@@ -37,8 +37,6 @@ const NonInjectedKubectlDownloadMirror = observer(({ userStore }: Dependencies)
       themeName="lens"
     />
   </section>
-
-
 ));
 
 export const KubectlDownloadMirror = withInjectables<Dependencies>(
diff --git a/src/features/preferences/renderer/preference-items/kubernetes/kubernetes-preference-page.injectable.tsx b/src/features/preferences/renderer/preference-items/kubernetes/kubernetes-preference-page.injectable.tsx
index 34a3aa4bac..d2fb8adfd1 100644
--- a/src/features/preferences/renderer/preference-items/kubernetes/kubernetes-preference-page.injectable.tsx
+++ b/src/features/preferences/renderer/preference-items/kubernetes/kubernetes-preference-page.injectable.tsx
@@ -4,8 +4,8 @@
  */
 import { getInjectable } from "@ogre-tools/injectable";
 import { preferenceItemInjectionToken } from "../preference-item-injection-token";
-import React from "react";
 import { getPreferencePage } from "../../get-preference-page";
+import { HorizontalLine } from "../../horizontal-line/horizontal-line";
 
 const kubernetesPreferencePageInjectable = getInjectable({
   id: "kubernetes-preference-page",
@@ -16,7 +16,7 @@ const kubernetesPreferencePageInjectable = getInjectable({
     parentId: "kubernetes-tab",
     orderNumber: 0,
     Component: getPreferencePage("Kubernetes"),
-    childrenSeparator: () => <hr />,
+    childrenSeparator: HorizontalLine,
   }),
 
   injectionToken: preferenceItemInjectionToken,
diff --git a/src/features/preferences/renderer/preference-items/preference-tab-root.tsx b/src/features/preferences/renderer/preference-items/preference-tab-root.tsx
index 1575357526..517729b225 100644
--- a/src/features/preferences/renderer/preference-items/preference-tab-root.tsx
+++ b/src/features/preferences/renderer/preference-items/preference-tab-root.tsx
@@ -4,7 +4,7 @@
  */
 import type { IComputedValue } from "mobx";
 import { computed } from "mobx";
-import React from "react";
+import { HorizontalLine } from "../horizontal-line/horizontal-line";
 
 export interface PreferenceTabsRoot {
   kind: "preference-tabs-root";
@@ -19,5 +19,5 @@ export const preferenceTabsRoot: PreferenceTabsRoot = {
   id: "preference-tabs",
   parentId: undefined,
   isShown: computed(() => true as const),
-  childrenSeparator: () => <hr />,
+  childrenSeparator: HorizontalLine,
 };
diff --git a/src/features/preferences/renderer/preference-items/proxy/proxy-preference-page.injectable.tsx b/src/features/preferences/renderer/preference-items/proxy/proxy-preference-page.injectable.tsx
index 9d6b48e007..1bd790ef74 100644
--- a/src/features/preferences/renderer/preference-items/proxy/proxy-preference-page.injectable.tsx
+++ b/src/features/preferences/renderer/preference-items/proxy/proxy-preference-page.injectable.tsx
@@ -6,6 +6,7 @@ import { getInjectable } from "@ogre-tools/injectable";
 import { preferenceItemInjectionToken } from "../preference-item-injection-token";
 import React from "react";
 import { getPreferencePage } from "../../get-preference-page";
+import { HorizontalLine } from "../../horizontal-line/horizontal-line";
 
 const proxyPreferencePageInjectable = getInjectable({
   id: "proxy-preference-page",
@@ -16,7 +17,7 @@ const proxyPreferencePageInjectable = getInjectable({
     parentId: "proxy-tab",
     orderNumber: 0,
     Component: getPreferencePage("Proxy"),
-    childrenSeparator: () => <hr className="small" />,
+    childrenSeparator: () => <HorizontalLine small />,
   }),
 
   injectionToken: preferenceItemInjectionToken,
diff --git a/src/features/preferences/renderer/preferences.tsx b/src/features/preferences/renderer/preferences.tsx
index cd719f0e18..5e0d38322e 100644
--- a/src/features/preferences/renderer/preferences.tsx
+++ b/src/features/preferences/renderer/preferences.tsx
@@ -15,39 +15,39 @@ import type { IComputedValue } from "mobx";
 import { Map } from "../../../renderer/components/map/map";
 import { observer } from "mobx-react";
 import { PreferencesNavigation } from "./preference-navigation/preferences-navigation";
+import Gutter from "../../../renderer/components/gutter/gutter";
 
 interface Dependencies {
   closePreferences: () => void;
   pageComposite: IComputedValue<Composite<PreferenceTab> | undefined>;
 }
 
-const NonInjectedPreferences = observer(({
-  closePreferences,
-  pageComposite,
-}: Dependencies) => {
-  const composite = pageComposite.get();
+const NonInjectedPreferences = observer(
+  ({ closePreferences, pageComposite }: Dependencies) => {
+    const composite = pageComposite.get();
 
-  return (
-    <SettingLayout
-      navigation={<PreferencesNavigation />}
-      className="Preferences"
-      contentGaps={false}
-      closeButtonProps={{ "data-testid": "close-preferences" }}
-      back={closePreferences}
-    >
-      {composite ? (
-        toPreferenceItemHierarchy(composite)
-      ) : (
-        <div
-          className="flex items-center"
-          data-preference-page-does-not-exist-test={true}
-        >
-          No preferences found
-        </div>
-      )}
-    </SettingLayout>
-  );
-});
+    return (
+      <SettingLayout
+        navigation={<PreferencesNavigation />}
+        className="Preferences"
+        contentGaps={false}
+        closeButtonProps={{ "data-testid": "close-preferences" }}
+        back={closePreferences}
+      >
+        {composite ? (
+          toPreferenceItemHierarchy(composite)
+        ) : (
+          <div
+            className="flex items-center"
+            data-preference-page-does-not-exist-test={true}
+          >
+            No preferences found
+          </div>
+        )}
+      </SettingLayout>
+    );
+  },
+);
 
 const toPreferenceItemHierarchy = (composite: Composite<PreferenceTypes>) => {
   const value = composite.value;
@@ -56,7 +56,10 @@ const toPreferenceItemHierarchy = (composite: Composite<PreferenceTypes>) => {
     case "group": {
       return (
         <section id={value.id}>
-          <Map items={composite.children} getSeparator={value.childrenSeparator}>
+          <Map
+            items={composite.children}
+            getSeparator={value.childrenSeparator || DefaultSeparator}
+          >
             {toPreferenceItemHierarchy}
           </Map>
         </section>
@@ -69,7 +72,10 @@ const toPreferenceItemHierarchy = (composite: Composite<PreferenceTypes>) => {
       return (
         <div data-preference-item-test={composite.id}>
           <Component>
-            <Map items={composite.children} getSeparator={value.childrenSeparator}>
+            <Map
+              items={composite.children}
+              getSeparator={value.childrenSeparator}
+            >
               {toPreferenceItemHierarchy}
             </Map>
           </Component>
@@ -83,7 +89,10 @@ const toPreferenceItemHierarchy = (composite: Composite<PreferenceTypes>) => {
 
       return (
         <Component item={value}>
-          <Map items={composite.children} getSeparator={value.childrenSeparator}>
+          <Map
+            items={composite.children}
+            getSeparator={value.childrenSeparator || DefaultSeparator}
+          >
             {toPreferenceItemHierarchy}
           </Map>
         </Component>
@@ -94,11 +103,7 @@ const toPreferenceItemHierarchy = (composite: Composite<PreferenceTypes>) => {
 
     // eslint-disable-next-line no-fallthrough
     case "tab": {
-      return (
-        <Map items={composite.children}>
-          {toPreferenceItemHierarchy}
-        </Map>
-      );
+      return <Map items={composite.children}>{toPreferenceItemHierarchy}</Map>;
     }
 
     default: {
@@ -109,7 +114,9 @@ const toPreferenceItemHierarchy = (composite: Composite<PreferenceTypes>) => {
       // Note: this code is unreachable, it is here to make ts not complain about
       // _exhaustiveCheck not being used.
       // See: https://www.typescriptlang.org/docs/handbook/2/narrowing.html#exhaustiveness-checking
-      throw new Error(`Tried to create preferences, but foreign item was encountered: ${_exhaustiveCheck} ${value}`);
+      throw new Error(
+        `Tried to create preferences, but foreign item was encountered: ${_exhaustiveCheck} ${value}`,
+      );
     }
   }
 };
@@ -126,3 +133,4 @@ export const Preferences = withInjectables<Dependencies>(
   },
 );
 
+const DefaultSeparator = () => <Gutter size="xl" />;
diff --git a/src/renderer/components/gutter/gutter.module.scss b/src/renderer/components/gutter/gutter.module.scss
index 8cf1567a66..2671090f19 100644
--- a/src/renderer/components/gutter/gutter.module.scss
+++ b/src/renderer/components/gutter/gutter.module.scss
@@ -12,3 +12,4 @@ $baseline: 8px;
 
 @include gutterSize('sm', 2 * $baseline);
 @include gutterSize('md', 3 * $baseline);
+@include gutterSize('xl', 5 * $baseline);
diff --git a/src/renderer/components/gutter/gutter.tsx b/src/renderer/components/gutter/gutter.tsx
index d19a82ddf8..c0fce2b6ba 100644
--- a/src/renderer/components/gutter/gutter.tsx
+++ b/src/renderer/components/gutter/gutter.tsx
@@ -7,7 +7,7 @@ import { cssNames } from "../../utils";
 import styles from "./gutter.module.scss";
 
 interface GutterProps {
-  size?: "sm" | "md";
+  size?: "sm" | "md" | "xl";
 }
 
 const Gutter = ({ size = "md" }: GutterProps) => {
diff --git a/src/renderer/components/layout/setting-layout.scss b/src/renderer/components/layout/setting-layout.scss
index 244b88b3dc..b37a68a569 100644
--- a/src/renderer/components/layout/setting-layout.scss
+++ b/src/renderer/components/layout/setting-layout.scss
@@ -121,14 +121,6 @@
     display: flex;
     flex-direction: column;
 
-    &:not(:first-of-type) {
-      margin-top: 40px;
-
-      &.small {
-        margin-top: 20px;
-      }
-    }
-
     h1, h2 {
       color: var(--textColorAccent);
     }
@@ -167,15 +159,14 @@
 
     hr {
       margin-top: 40px;
+      margin-bottom: 40px;
+
       height: 1px;
       border-top: thin solid var(--hrColor);
 
       &.small {
         margin-top: 20px;
-      }
-
-      &:last-child {
-        display: none;
+        margin-bottom: 20px;
       }
     }