From 117421b4cba19ccde7a584f34bb0f08f702c7121 Mon Sep 17 00:00:00 2001
From: Paul Lessing <paul@paullessing.com>
Date: Wed, 29 Jun 2022 08:33:07 +0100
Subject: [PATCH] Feat: Only show save domains button if changes were made

The logic for this is that we show the button if either:
a) one or more new non-empty domains have been added, or
b) an existing domain was modified, even if the modification was then undone.

The reasoning for b) is as follows:
If a user adds a new domain row, makes changes, then removes the domain row, it is clear to the user that no changes have been made, and therefore the "save" button should not be visible.
However, as soon as the user makes any changes to an existing domain, they want to feel confident that they can hit save and ensure that whatever change they made is persisted; even if the change is identical to the current state, because they may not be able to recall accurately what the current state was. In those situations a user gets more confidence out of being able to hit save, than they would from being told by the system "you haven't made any changes".
---
 app/scenes/Settings/Security.tsx | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/app/scenes/Settings/Security.tsx b/app/scenes/Settings/Security.tsx
index f56d2ab2d..19b98ca3d 100644
--- a/app/scenes/Settings/Security.tsx
+++ b/app/scenes/Settings/Security.tsx
@@ -41,6 +41,16 @@ function Security() {
   const [allowedDomains, setAllowedDomains] = useState([
     ...(team.allowedDomains ?? []),
   ]);
+  const [lastKnownDomainCount, updateLastKnownDomainCount] = useState(
+    allowedDomains.length
+  );
+
+  const [existingDomainsTouched, setExistingDomainsTouched] = useState(false);
+
+  const showSaveDomainsButton = React.useCallback(() => {
+    const validDomains = allowedDomains.filter((value) => value !== "");
+    return existingDomainsTouched || validDomains.length > lastKnownDomainCount;
+  }, [existingDomainsTouched, allowedDomains, lastKnownDomainCount]);
 
   const authenticationMethods = team.signinMethods;
 
@@ -54,8 +64,6 @@ function Security() {
     [showToast, t]
   );
 
-  const [domainsChanged, setDomainsChanged] = useState(false);
-
   const saveData = React.useCallback(
     async (newData) => {
       try {
@@ -84,9 +92,9 @@ function Security() {
         allowedDomains,
       });
       showSuccessMessage();
-      setDomainsChanged(false);
+      setExistingDomainsTouched(false);
+      updateLastKnownDomainCount(allowedDomains.length);
     } catch (err) {
-      setDomainsChanged(true);
       showToast(err.message, {
         type: "error",
       });
@@ -142,14 +150,17 @@ function Security() {
     const newDomains = allowedDomains.filter((_, i) => index !== i);
 
     setAllowedDomains(newDomains);
-    setDomainsChanged(true);
+
+    const touchedExistingDomain = index < lastKnownDomainCount;
+    if (touchedExistingDomain) {
+      setExistingDomainsTouched(true);
+    }
   };
 
   const handleAddDomain = () => {
     const newDomains = [...allowedDomains, ""];
 
     setAllowedDomains(newDomains);
-    setDomainsChanged(true);
   };
 
   const createOnDomainChangedHandler = (index: number) => (
@@ -159,7 +170,11 @@ function Security() {
 
     newDomains[index] = ev.currentTarget.value;
     setAllowedDomains(newDomains);
-    setDomainsChanged(true);
+
+    const touchedExistingDomain = index < lastKnownDomainCount;
+    if (touchedExistingDomain) {
+      setExistingDomainsTouched(true);
+    }
   };
 
   return (
@@ -318,7 +333,7 @@ function Security() {
             <span />
           )}
 
-          {domainsChanged && (
+          {showSaveDomainsButton() && (
             <Fade>
               <Button
                 type="button"