Consolidate button wording in dialogs (“Close” / “Back”) (#1688)

Related #1684. Stacked onto #1687. As discussed in tiny-pilot/tinypilot-pro#1139, this PR consolidates the wording for “Cancel”/“Close”/“Back”/“OK” buttons across all our dialogs, to only use “Close” or “Back”. It also outlines the rules in the style guide. - In the style guide, I restructured some of the existing text, to accommodate the new rules in a clearer way. - The code for the demo overlays has become borderline complex. To me, it’s still okay-ish for now, but for the future we could consider to simplify the setup, and make it more straightforward / less interactive. - I didn’t change the `id` attributes or CSS class names of the affected button elements, so their wording might now diverge from the label: e.g., we have a button labelled `Close`, with a `cancel-hostname-change` id attribute. To me, the UI label and the internal technical name are conceptually different enough that it wouldn’t be worth the hassle to keep them aligned, and we already diverge in other places anyway (e.g., we have a button [labelled `Paste`, with a `confirm-btn` id attribute](https://github.com/tiny-pilot/tinypilot/blob/a1aa0249bc64c8dd1ee4c75e7ea0daa982bb2d22/app/templates/custom-elements/paste-dialog.html#L30); or a button [labelled `Apply`, with a `save-btn` id attribute](https://github.com/tiny-pilot/tinypilot/blob/a1aa0249bc64c8dd1ee4c75e7ea0daa982bb2d22/app/templates/custom-elements/video-settings-dialog.html#L190).) - If we’d feel that it’s worth to unify all button labels with their `id` attribute / CSS class name, I’d suggest to do that in a separate refactoring step, and for all buttons in one go. https://github.com/tiny-pilot/tinypilot/assets/83721279/e426c8db-b449-4e5f-968c-1d1bbcc129eb ~Currently blocked by #1686 (e2e tests failing).~ <a data-ca-tag href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1688"><img src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review on CodeApprove" /></a> --------- Co-authored-by: Jan Heuermann <[email protected]>
tiny-pilot · Nov 22, 2023 · 5bdbec7 · 5bdbec7
1 parent 59025c7
commit 5bdbec7
Show file tree

Hide file tree

Showing 8 changed files with 90 additions and 29 deletions.
diff --git a/app/templates/custom-elements/change-hostname-dialog.html b/app/templates/custom-elements/change-hostname-dialog.html
@@ -65,7 +65,7 @@ <h3>Change Hostname</h3>
     <button id="change-and-restart" class="btn-success" type="button">
       Change and Restart
     </button>
-    <button id="cancel-hostname-change" type="button">Cancel</button>
+    <button id="cancel-hostname-change" type="button">Close</button>
   </div>
 
   <div id="changing">

diff --git a/app/templates/custom-elements/feature-pro-dialog.html b/app/templates/custom-elements/feature-pro-dialog.html
@@ -14,7 +14,7 @@ <h3>This Feature is Available in TinyPilot Pro</h3>
     >
       Learn More
     </a>
-    <button id="cancel-upgrade-to-pro" type="button">Cancel</button>
+    <button id="cancel-upgrade-to-pro" type="button">Close</button>
   </div>
 </template>
 

diff --git a/app/templates/custom-elements/paste-dialog.html b/app/templates/custom-elements/paste-dialog.html
@@ -28,7 +28,7 @@ <h3>Paste Text</h3>
     spellcheck="false"
   ></textarea>
   <button id="confirm-btn" class="btn-success">Paste</button>
-  <button id="cancel-btn">Cancel</button>
+  <button id="cancel-btn">Close</button>
 </template>
 
 <script type="module">

diff --git a/app/templates/custom-elements/shutdown-dialog.html b/app/templates/custom-elements/shutdown-dialog.html
@@ -39,7 +39,7 @@ <h3>Shut Down TinyPilot Device?</h3>
     <button id="confirm-restart" class="btn-danger" type="button">
       Restart
     </button>
-    <button id="cancel-shutdown" type="button">Cancel</button>
+    <button id="cancel-shutdown" type="button">Close</button>
   </div>
 
   <div id="restarting">

diff --git a/app/templates/custom-elements/update-dialog.html b/app/templates/custom-elements/update-dialog.html
@@ -83,7 +83,7 @@ <h3>Update TinyPilot</h3>
   <div id="latest">
     <h3>No Updates Available</h3>
     <p>You are running the latest version of TinyPilot.</p>
-    <button id="ok-latest" type="button">OK</button>
+    <button id="ok-latest" type="button">Close</button>
   </div>
 
   <div id="update-container">
@@ -102,7 +102,7 @@ <h3>Restarting to Complete Update</h3>
 
     <div id="update-finished">
       <h3>Update Complete</h3>
-      <button id="ok-finished" type="button">OK</button>
+      <button id="ok-finished" type="button">Close</button>
     </div>
 
     <details>

diff --git a/app/templates/custom-elements/update-prompt-automatic.html b/app/templates/custom-elements/update-prompt-automatic.html
@@ -6,7 +6,7 @@
 
   <p>An update is available. Would you like to install the latest version?</p>
   <button id="confirm-update" class="btn-success" type="button">Update</button>
-  <button id="cancel-update" type="button">Cancel</button>
+  <button id="cancel-update" type="button">Close</button>
 </template>
 
 <script type="module">

diff --git a/app/templates/custom-elements/video-settings-dialog.html b/app/templates/custom-elements/video-settings-dialog.html
@@ -188,7 +188,7 @@ <h4>Advanced Settings</h4>
       </inline-message>
     </div>
     <button class="save-btn btn-success" type="button">Apply</button>
-    <button class="close-btn" type="button">Cancel</button>
+    <button class="close-btn" type="button">Close</button>
   </div>
   <div id="saving">
     <h3>Applying Video Settings</h3>

diff --git a/app/templates/styleguide.html b/app/templates/styleguide.html
@@ -371,16 +371,21 @@ <h3>Design</h3>
       </button>
       <overlay-panel id="overlay-with-primary-action">
         <h3>Overlay with Primary Action</h3>
-        <div>
+        <p>
           This overlay style is used for linear and straightforward flows that
-          have a single purpose or end state, like a prompt or confirmation. It
-          contains a call-to-action button and a cancel button (in that order)
-          at the bottom. When clicked, they also terminate the dialog.
-        </div>
+          have a single purpose or end state, like a prompt or confirmation.
+        </p>
+        <p>
+          In the bottom row, the overlay should contain a call-to-action button,
+          and a button for aborting the flow.<br />
+          The button for aborting the flow should be labelled “Close”, and it
+          should be the rightmost one. We deliberately don’t say “Cancel”, to be
+          transparent that clicking the button will terminate the overlay.
+        </p>
         <button class="close-overlay-with-primary-action btn-success">
           Confirm
         </button>
-        <button class="close-overlay-with-primary-action">Cancel</button>
+        <button class="close-overlay-with-primary-action">Close</button>
       </overlay-panel>
       <script type="module">
         import { DialogClosedEvent } from "/js/events.js";
@@ -400,35 +405,95 @@ <h3>Overlay with Primary Action</h3>
       </script>
       <!-- Overlay with feature: -->
       <button id="show-overlay-with-feature-btn" class="btn-action">
-        Overlay with Feature
+        Overlay with Feature Panel
       </button>
       <overlay-panel id="overlay-with-feature">
-        <h3>Overlay with Feature</h3>
-        <div>
-          <button class="btn-action" onclick="alert('Done.')">Action 1</button>
-          <button class="btn-action" onclick="alert('Done.')">Action 2</button>
-          <button class="btn-action" onclick="alert('Done.')">Action 3</button>
+        <div id="overlay-with-feature-initial-state">
+          <h3>Overlay with Feature Panel</h3>
+          <div>
+            <button class="btn-action" onclick="alert('I’m an alert!')">
+              Show an Alert
+            </button>
+            <button
+              class="btn-action"
+              id="overlay-with-feature-secondary-state-btn"
+            >
+              Go to Secondary State
+            </button>
+          </div>
+          <p>
+            This overlay style is used for more comprehensive, non-linear
+            features.
+          </p>
+          <p>
+            In the bottom row, the overlay should contain a single button, which
+            is labelled “Close” and terminates the dialog.
+          </p>
+          <p>
+            The overlay may contain “secondary” actions that advance the overlay
+            into a different state. The corresponding buttons shouldn’t be
+            primary ones, and they shouldn’t appear in the bottom row.
+          </p>
+          <button id="close-overlay-with-feature-btn">Close</button>
         </div>
-        <div>
-          When the user can carry out “secondary” actions within the overlay
-          that don’t terminate the dialog itself, the blue action buttons should
-          be used for that. They also shouldn’t appear at the bottom.
+        <div id="overlay-with-feature-secondary-state">
+          <h3>Secondary State</h3>
+          <p>
+            The secondary state should have buttons in the bottom row, which
+            follow the same rules as in the “Overlay with Primary Action”,
+            except that …:
+          </p>
+          <ul style="text-align: left">
+            <li>
+              The buttons take the user back to the initial overlay state
+              instead of terminating the overlay.
+            </li>
+            <li>
+              The rightmost button is labelled “Back” instead of “Close”. We
+              deliberately don’t say “Cancel”, as that would make it unclear
+              whether the button terminates the overlay.
+            </li>
+          </ul>
+          <button class="btn-success overlay-with-feature-initialize-btn">
+            Confirm
+          </button>
+          <button class="overlay-with-feature-initialize-btn">Back</button>
         </div>
-        <button id="close-overlay-with-feature-btn">Close</button>
       </overlay-panel>
       <script type="module">
         import { DialogClosedEvent } from "/js/events.js";
 
+        function setDialogState(newState /* = "initial" or "secondary" */) {
+          document.getElementById(
+            "overlay-with-feature-initial-state"
+          ).style.display = newState === "initial" ? "block" : "none";
+          document.getElementById(
+            "overlay-with-feature-secondary-state"
+          ).style.display = newState === "initial" ? "none" : "block";
+        }
         document
           .getElementById("show-overlay-with-feature-btn")
           .addEventListener("click", () => {
+            setDialogState("initial");
             document.getElementById("overlay-with-feature").show();
           });
         document
           .getElementById("close-overlay-with-feature-btn")
           .addEventListener("click", (evt) => {
             evt.target.dispatchEvent(new DialogClosedEvent());
           });
+        document
+          .getElementById("overlay-with-feature-secondary-state-btn")
+          .addEventListener("click", () => {
+            setDialogState("secondary");
+          });
+        document
+          .querySelectorAll(".overlay-with-feature-initialize-btn")
+          .forEach((el) =>
+            el.addEventListener("click", () => {
+              setDialogState("initial");
+            })
+          );
       </script>
       <!-- Error overlay: -->
       <overlay-panel id="error-overlay" variant="danger">
@@ -480,10 +545,6 @@ <h3>Primary Actions</h3>
         dialog. These primary actions should either advance the dialog into a
         different state, or they should terminate the dialog altogether.
       </p>
-      <p>
-        If there is a primary button for closing or canceling the dialog, it
-        should always be the rightmost one.
-      </p>
 
       <h2 class="section">Progress Indicator</h2>
       <p>