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>

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;
+}

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,
+    })}
+  />
+);

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,

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>(

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,

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,
 };

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,

src/features/preferences/renderer/preferences.tsx

@@ -15,16 +15,15 @@ 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(({
+const NonInjectedPreferences = observer(
-  closePreferences,
+  ({ closePreferences, pageComposite }: Dependencies) => {
-  pageComposite,
-}: Dependencies) => {
     const composite = pageComposite.get();
 
     return (
@@ -47,7 +46,8 @@ const NonInjectedPreferences = observer(({
         )}
       </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 (
+      return <Map items={composite.children}>{toPreferenceItemHierarchy}</Map>;
-        <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" />;

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);

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) => {

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;
-      }
+        margin-bottom: 20px;
-
-      &:last-child {
-        display: none;
       }
     }