Merge pull request #1429 from huwshimi/fix-config-mobile-expand

WD-2670 - Fix config descriptions

src/panels/ConfigPanel/ConfigField.tsx

@@ -1,8 +1,16 @@
-import { ReactNode, useEffect, useRef, useState } from "react";
+import {
+  ReactNode,
+  useCallback,
+  useEffect,
+  useMemo,
+  useRef,
+  useState,
+} from "react";
 import classnames from "classnames";
 
 import { isSet } from "components/utils";
 import { ConfigData } from "juju/api";
+import { Button, Icon } from "@canonical/react-components";
 
 export type SetNewValue = (name: string, value: any) => void;
 
@@ -31,7 +39,7 @@ const ConfigField = <V,>({
   );
   const [showDescription, setShowDescription] = useState(false);
   const descriptionRef = useRef<HTMLDivElement>(null);
-  const [maxDescriptionHeight, setMaxDescriptionHeight] = useState("0px");
+  const [maxDescriptionHeight, setMaxDescriptionHeight] = useState<string>();
 
   let inputValue = config.default;
   if (isSet(config.newValue)) {
@@ -40,22 +48,59 @@ const ConfigField = <V,>({
     inputValue = config.value;
   }
 
-  useEffect(() => {
-    if (descriptionRef.current?.firstChild) {
+  const updateDescriptionHeight = useCallback(() => {
+    if (
+      // Don't update if the height has already been retrieved.
+      !maxDescriptionHeight &&
+      descriptionRef.current?.firstChild &&
+      // Don't try and update if the element is not visible.
+      descriptionRef.current?.offsetParent !== null
+    ) {
       setMaxDescriptionHeight(
         `${
           (descriptionRef.current.firstChild as HTMLPreElement).clientHeight
         }px`
       );
     }
-  }, []);
+  }, [maxDescriptionHeight]);
+
+  const resizeObserver = useMemo(
+    () => new ResizeObserver(updateDescriptionHeight),
+    [updateDescriptionHeight]
+  );
+
+  useEffect(() => {
+    if (maxDescriptionHeight) {
+      // There's no need to keep observing the element once the height has
+      // been retrieved.
+      resizeObserver.disconnect();
+    }
+  }, [maxDescriptionHeight, resizeObserver]);
+
+  useEffect(() => {
+    const descriptionElement = descriptionRef.current;
+    if (!descriptionElement) {
+      return;
+    }
+    // Attempt to set the height if the element is visible.
+    updateDescriptionHeight();
+    // On larger screens the description is hidden so the element needs to be
+    // observed for when the screen is resized down and it becomes visible so
+    // that the height can be retrieved.
+    resizeObserver.observe(descriptionElement);
+    return () => {
+      if (descriptionElement) {
+        resizeObserver.unobserve(descriptionElement);
+      }
+    };
+  }, [updateDescriptionHeight, resizeObserver]);
 
   useEffect(() => {
     if (!descriptionRef.current) {
       return;
     }
     if (showDescription) {
-      descriptionRef.current.style.maxHeight = maxDescriptionHeight;
+      descriptionRef.current.style.maxHeight = maxDescriptionHeight ?? "0px";
     } else {
       descriptionRef.current.style.maxHeight = "0px";
     }
@@ -102,16 +147,15 @@ const ConfigField = <V,>({
       onClick={() => setSelectedConfig(config)}
     >
       <h5 className="u-float-left">
-        <i
-          className={classnames("config-input--view-description", {
-            "p-icon--plus": !showDescription,
-            "p-icon--minus": showDescription,
-          })}
-          onClick={handleShowDescription}
-          onKeyPress={handleShowDescription}
-          role="button"
-          tabIndex={0}
-        />
+        {config.description ? (
+          <Button
+            appearance="base"
+            className="config-input--view-description"
+            onClick={handleShowDescription}
+          >
+            <Icon name={showDescription ? "minus" : "plus"} />
+          </Button>
+        ) : null}
         {config.name}
       </h5>
       <button

src/panels/ConfigPanel/_config-panel.scss

@@ -223,11 +223,16 @@ $panel-padding: 1.5rem;
     background-color: $color-light;
   }
 
-  .p-icon--plus.config-input--view-description,
-  .p-icon--minus.config-input--view-description {
+  .config-input--view-description {
     display: none;
-    margin-right: 0.5rem;
-    width: 0.875rem;
+    // Give the toggle button a larger hit area:
+    margin: -$spv--x-small 0 0 (-$spv--small);
+    padding: $spv--x-small $spv--small;
+
+    .p-icon--minus,
+    .p-icon--plus {
+      width: #{map-get($font-sizes, small)}rem;
+    }
 
     @media screen and (max-width: $breakpoint-large) {
       display: inline-block;

src/setupTests.js

@@ -18,7 +18,14 @@ if (!window.HTMLDivElement.prototype.animate) {
   );
 }
 
+class ResizeObserver {
+  observe = jest.fn();
+  unobserve = jest.fn();
+  disconnect = jest.fn();
+}
+
 // Provide node modules that are required by bakeryjs.
 global.TextDecoder = util.TextDecoder;
 global.TextEncoder = util.TextEncoder;
 global.crypto = crypto;
+global.ResizeObserver = ResizeObserver;