From 464eb93a44a76f2d944a4d67c9f48becb194a0b8 Mon Sep 17 00:00:00 2001
From: Suguru Hirahara <luixxiul@users.noreply.github.com>
Date: Wed, 11 May 2022 12:33:21 +0000
Subject: [PATCH] Fix visual bugs on AccessSecretStorageDialog (#8160)

* Remove duplicate mx_Dialog_buttons

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Group buttons on mx_Dialog with span

- Cancel default styling for elements inside .mx_AccessSecretStorageDialog_primaryContainer

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Move common rules of mx_Dialog_buttons_row to _common.scss

- Set 16px gap between buttons per https://github.com/vector-im/element-web/issues/19426#issuecomment-949778515

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Spacing variables

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Nesting - .mx_AccessSecretStorageDialog_reset

- Fix the reset link color
- Set $spacingStart variable

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Remove unnecessary rule

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
---
 res/css/_common.scss                          |  20 +-
 .../security/_AccessSecretStorageDialog.scss  |  73 +++--
 .../security/CreateKeyBackupDialog.tsx        |  13 +-
 .../views/elements/DialogButtons.tsx          |  24 +-
 .../__snapshots__/ExportDialog-test.tsx.snap  | 304 ++++++++++--------
 5 files changed, 255 insertions(+), 179 deletions(-)

diff --git a/res/css/_common.scss b/res/css/_common.scss
index 9ef9720b6f..ade69bc941 100644
--- a/res/css/_common.scss
+++ b/res/css/_common.scss
@@ -20,6 +20,7 @@ limitations under the License.
 @import "./_font-sizes.scss";
 @import "./_font-weights.scss";
 @import "./_animations.scss";
+@import "./_spacing.scss";
 @import url("maplibre-gl/dist/maplibre-gl.css");
 
 $hover-transition: 0.08s cubic-bezier(.46, .03, .52, .96); // quadratic
@@ -414,7 +415,8 @@ legend {
 }
 
 .mx_Dialog_buttons {
-    margin-top: 20px;
+    margin-top: $spacing-20;
+    margin-inline-start: auto;
     text-align: right;
 
     .mx_Dialog_buttons_additive {
@@ -423,6 +425,22 @@ legend {
     }
 }
 
+.mx_Dialog_buttons_row {
+    display: flex;
+    flex-wrap: wrap;
+    justify-content: flex-end;
+    text-align: initial;
+    margin-inline-start: auto;
+
+    // default gap among elements
+    column-gap: $spacing-8; // See margin-right below inside the button style
+    row-gap: 5px; // See margin-bottom below inside the button style
+
+    button {
+        margin: 0 !important; // override the margin settings
+    }
+}
+
 /* XXX: Our button style are a mess: buttons that happen to appear in dialogs get special styles applied
  * to them that no button anywhere else in the app gets by default. In practice, buttons in other places
  * in the app look the same by being AccessibleButtons, or possibly by having explict button classes.
diff --git a/res/css/views/dialogs/security/_AccessSecretStorageDialog.scss b/res/css/views/dialogs/security/_AccessSecretStorageDialog.scss
index 609c022f85..5756290505 100644
--- a/res/css/views/dialogs/security/_AccessSecretStorageDialog.scss
+++ b/res/css/views/dialogs/security/_AccessSecretStorageDialog.scss
@@ -14,38 +14,14 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-.mx_AccessSecretStorageDialog_reset {
-    position: relative;
-    padding-left: 24px; // 16px icon + 8px padding
-    margin-top: 7px; // vertical alignment to buttons
-    margin-bottom: 7px; // space between the buttons and the text when float is activated
-    text-align: left;
-
-    &::before {
-        content: "";
-        display: inline-block;
-        position: absolute;
-        height: 16px;
-        width: 16px;
-        left: 0;
-        top: 2px; // alignment
-        background-image: url("$(res)/img/element-icons/warning-badge.svg");
-        background-size: contain;
-    }
-
-    .mx_AccessSecretStorageDialog_reset_link {
-        color: $alert;
-    }
-}
-
 .mx_AccessSecretStorageDialog_titleWithIcon::before {
     content: '';
     display: inline-block;
     width: 24px;
     height: 24px;
-    margin-right: 8px;
+    margin-inline-end: $spacing-8;
     position: relative;
-    top: 5px;
+    top: 5px; // TODO: spacing variable
     background-color: $primary-content;
 }
 
@@ -84,7 +60,7 @@ limitations under the License.
 }
 
 .mx_AccessSecretStorageDialog_recoveryKeyEntry_entryControlSeparatorText {
-    margin: 16px;
+    margin: $spacing-16;
 }
 
 .mx_AccessSecretStorageDialog_recoveryKeyFeedback {
@@ -97,7 +73,7 @@ limitations under the License.
         mask-repeat: no-repeat;
         mask-position: center;
         mask-size: 20px;
-        margin-right: 5px;
+        margin-inline-end: 5px; // TODO: spacing variable
     }
 }
 
@@ -120,3 +96,44 @@ limitations under the License.
 .mx_AccessSecretStorageDialog_recoveryKeyEntry_fileInput {
     display: none;
 }
+
+.mx_AccessSecretStorageDialog_primaryContainer {
+    .mx_Dialog_buttons {
+        $spacingStart: $spacing-24;  // 16px icon + 8px padding
+
+        text-align: initial;
+        display: flex;
+        flex-flow: column;
+        gap: 14px;  // TODO: spacing variable
+
+        .mx_Dialog_buttons_additive {
+            float: none;
+
+            .mx_AccessSecretStorageDialog_reset {
+                position: relative;
+                padding-inline-start: $spacingStart;
+
+                &::before {
+                    content: "";
+                    display: inline-block;
+                    position: absolute;
+                    height: 16px;
+                    width: 16px;
+                    left: 0;
+                    top: 2px; // alignment
+                    background-image: url("$(res)/img/element-icons/warning-badge.svg");
+                    background-size: contain;
+                }
+
+                .mx_AccessSecretStorageDialog_reset_link {
+                    color: $alert;
+                }
+            }
+        }
+
+        .mx_Dialog_buttons_row {
+            gap: $spacing-16; // TODO: needs normalization
+            padding-inline-start: $spacingStart;
+        }
+    }
+}
diff --git a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx
index dea6c87941..707e3cbaf7 100644
--- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx
+++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx
@@ -455,13 +455,12 @@ export default class CreateKeyBackupDialog extends React.PureComponent<IProps, I
         if (this.state.error) {
             content = <div>
                 <p>{ _t("Unable to create key backup") }</p>
-                <div className="mx_Dialog_buttons">
-                    <DialogButtons primaryButton={_t('Retry')}
-                        onPrimaryButtonClick={this.createBackup}
-                        hasCancel={true}
-                        onCancel={this.onCancel}
-                    />
-                </div>
+                <DialogButtons
+                    primaryButton={_t('Retry')}
+                    onPrimaryButtonClick={this.createBackup}
+                    hasCancel={true}
+                    onCancel={this.onCancel}
+                />
             </div>;
         } else {
             switch (this.state.phase) {
diff --git a/src/components/views/elements/DialogButtons.tsx b/src/components/views/elements/DialogButtons.tsx
index 2a3554e876..bf018e14f4 100644
--- a/src/components/views/elements/DialogButtons.tsx
+++ b/src/components/views/elements/DialogButtons.tsx
@@ -100,17 +100,19 @@ export default class DialogButtons extends React.Component<IProps> {
         return (
             <div className="mx_Dialog_buttons">
                 { additive }
-                { cancelButton }
-                { this.props.children }
-                <button type={this.props.primaryIsSubmit ? 'submit' : 'button'}
-                    data-test-id="dialog-primary-button"
-                    className={primaryButtonClassName}
-                    onClick={this.props.onPrimaryButtonClick}
-                    autoFocus={this.props.focus}
-                    disabled={this.props.disabled || this.props.primaryDisabled}
-                >
-                    { this.props.primaryButton }
-                </button>
+                <span className="mx_Dialog_buttons_row">
+                    { cancelButton }
+                    { this.props.children }
+                    <button type={this.props.primaryIsSubmit ? 'submit' : 'button'}
+                        data-test-id="dialog-primary-button"
+                        className={primaryButtonClassName}
+                        onClick={this.props.onPrimaryButtonClick}
+                        autoFocus={this.props.focus}
+                        disabled={this.props.disabled || this.props.primaryDisabled}
+                    >
+                        { this.props.primaryButton }
+                    </button>
+                </span>
             </div>
         );
     }
diff --git a/test/components/views/dialogs/__snapshots__/ExportDialog-test.tsx.snap b/test/components/views/dialogs/__snapshots__/ExportDialog-test.tsx.snap
index f97344beff..b922d8b8ba 100644
--- a/test/components/views/dialogs/__snapshots__/ExportDialog-test.tsx.snap
+++ b/test/components/views/dialogs/__snapshots__/ExportDialog-test.tsx.snap
@@ -245,19 +245,23 @@ Array [
               <div
                 class="mx_Dialog_buttons"
               >
-                <button
-                  data-test-id="dialog-cancel-button"
-                  type="button"
+                <span
+                  class="mx_Dialog_buttons_row"
                 >
-                  Cancel
-                </button>
-                <button
-                  class="mx_Dialog_primary"
-                  data-test-id="dialog-primary-button"
-                  type="button"
-                >
-                  Export
-                </button>
+                  <button
+                    data-test-id="dialog-cancel-button"
+                    type="button"
+                  >
+                    Cancel
+                  </button>
+                  <button
+                    class="mx_Dialog_primary"
+                    data-test-id="dialog-primary-button"
+                    type="button"
+                  >
+                    Export
+                  </button>
+                </span>
               </div>
             </div>
           }
@@ -466,19 +470,23 @@ Array [
                 <div
                   class="mx_Dialog_buttons"
                 >
-                  <button
-                    data-test-id="dialog-cancel-button"
-                    type="button"
+                  <span
+                    class="mx_Dialog_buttons_row"
                   >
-                    Cancel
-                  </button>
-                  <button
-                    class="mx_Dialog_primary"
-                    data-test-id="dialog-primary-button"
-                    type="button"
-                  >
-                    Export
-                  </button>
+                    <button
+                      data-test-id="dialog-cancel-button"
+                      type="button"
+                    >
+                      Cancel
+                    </button>
+                    <button
+                      class="mx_Dialog_primary"
+                      data-test-id="dialog-primary-button"
+                      type="button"
+                    >
+                      Export
+                    </button>
+                  </span>
                 </div>
               </div>
             }
@@ -815,22 +823,26 @@ Array [
             <div
               className="mx_Dialog_buttons"
             >
-              <button
-                data-test-id="dialog-cancel-button"
-                disabled={false}
-                onClick={[Function]}
-                type="button"
+              <span
+                className="mx_Dialog_buttons_row"
               >
-                Cancel
-              </button>
-              <button
-                className="mx_Dialog_primary"
-                data-test-id="dialog-primary-button"
-                onClick={[Function]}
-                type="button"
-              >
-                Export
-              </button>
+                <button
+                  data-test-id="dialog-cancel-button"
+                  disabled={false}
+                  onClick={[Function]}
+                  type="button"
+                >
+                  Cancel
+                </button>
+                <button
+                  className="mx_Dialog_primary"
+                  data-test-id="dialog-primary-button"
+                  onClick={[Function]}
+                  type="button"
+                >
+                  Export
+                </button>
+              </span>
             </div>
           </DialogButtons>
         </div>
@@ -1086,19 +1098,23 @@ Array [
             <div
               class="mx_Dialog_buttons"
             >
-              <button
-                data-test-id="dialog-cancel-button"
-                type="button"
+              <span
+                class="mx_Dialog_buttons_row"
               >
-                Cancel
-              </button>
-              <button
-                class="mx_Dialog_primary"
-                data-test-id="dialog-primary-button"
-                type="button"
-              >
-                Export
-              </button>
+                <button
+                  data-test-id="dialog-cancel-button"
+                  type="button"
+                >
+                  Cancel
+                </button>
+                <button
+                  class="mx_Dialog_primary"
+                  data-test-id="dialog-primary-button"
+                  type="button"
+                >
+                  Export
+                </button>
+              </span>
             </div>
           </div>
         }
@@ -1307,19 +1323,23 @@ Array [
               <div
                 class="mx_Dialog_buttons"
               >
-                <button
-                  data-test-id="dialog-cancel-button"
-                  type="button"
+                <span
+                  class="mx_Dialog_buttons_row"
                 >
-                  Cancel
-                </button>
-                <button
-                  class="mx_Dialog_primary"
-                  data-test-id="dialog-primary-button"
-                  type="button"
-                >
-                  Export
-                </button>
+                  <button
+                    data-test-id="dialog-cancel-button"
+                    type="button"
+                  >
+                    Cancel
+                  </button>
+                  <button
+                    class="mx_Dialog_primary"
+                    data-test-id="dialog-primary-button"
+                    type="button"
+                  >
+                    Export
+                  </button>
+                </span>
               </div>
             </div>
           }
@@ -1656,22 +1676,26 @@ Array [
           <div
             className="mx_Dialog_buttons"
           >
-            <button
-              data-test-id="dialog-cancel-button"
-              disabled={false}
-              onClick={[Function]}
-              type="button"
+            <span
+              className="mx_Dialog_buttons_row"
             >
-              Cancel
-            </button>
-            <button
-              className="mx_Dialog_primary"
-              data-test-id="dialog-primary-button"
-              onClick={[Function]}
-              type="button"
-            >
-              Export
-            </button>
+              <button
+                data-test-id="dialog-cancel-button"
+                disabled={false}
+                onClick={[Function]}
+                type="button"
+              >
+                Cancel
+              </button>
+              <button
+                className="mx_Dialog_primary"
+                data-test-id="dialog-primary-button"
+                onClick={[Function]}
+                type="button"
+              >
+                Export
+              </button>
+            </span>
           </div>
         </DialogButtons>
       </div>
@@ -1914,19 +1938,23 @@ Array [
           <div
             class="mx_Dialog_buttons"
           >
-            <button
-              data-test-id="dialog-cancel-button"
-              type="button"
+            <span
+              class="mx_Dialog_buttons_row"
             >
-              Cancel
-            </button>
-            <button
-              class="mx_Dialog_primary"
-              data-test-id="dialog-primary-button"
-              type="button"
-            >
-              Export
-            </button>
+              <button
+                data-test-id="dialog-cancel-button"
+                type="button"
+              >
+                Cancel
+              </button>
+              <button
+                class="mx_Dialog_primary"
+                data-test-id="dialog-primary-button"
+                type="button"
+              >
+                Export
+              </button>
+            </span>
           </div>
         </div>
       }
@@ -2135,19 +2163,23 @@ Array [
             <div
               class="mx_Dialog_buttons"
             >
-              <button
-                data-test-id="dialog-cancel-button"
-                type="button"
+              <span
+                class="mx_Dialog_buttons_row"
               >
-                Cancel
-              </button>
-              <button
-                class="mx_Dialog_primary"
-                data-test-id="dialog-primary-button"
-                type="button"
-              >
-                Export
-              </button>
+                <button
+                  data-test-id="dialog-cancel-button"
+                  type="button"
+                >
+                  Cancel
+                </button>
+                <button
+                  class="mx_Dialog_primary"
+                  data-test-id="dialog-primary-button"
+                  type="button"
+                >
+                  Export
+                </button>
+              </span>
             </div>
           </div>
         }
@@ -2484,22 +2516,26 @@ Array [
         <div
           className="mx_Dialog_buttons"
         >
-          <button
-            data-test-id="dialog-cancel-button"
-            disabled={false}
-            onClick={[Function]}
-            type="button"
+          <span
+            className="mx_Dialog_buttons_row"
           >
-            Cancel
-          </button>
-          <button
-            className="mx_Dialog_primary"
-            data-test-id="dialog-primary-button"
-            onClick={[Function]}
-            type="button"
-          >
-            Export
-          </button>
+            <button
+              data-test-id="dialog-cancel-button"
+              disabled={false}
+              onClick={[Function]}
+              type="button"
+            >
+              Cancel
+            </button>
+            <button
+              className="mx_Dialog_primary"
+              data-test-id="dialog-primary-button"
+              onClick={[Function]}
+              type="button"
+            >
+              Export
+            </button>
+          </span>
         </div>
       </DialogButtons>
     </div>
@@ -2833,22 +2869,26 @@ Array [
       <div
         className="mx_Dialog_buttons"
       >
-        <button
-          data-test-id="dialog-cancel-button"
-          disabled={false}
-          onClick={[Function]}
-          type="button"
+        <span
+          className="mx_Dialog_buttons_row"
         >
-          Cancel
-        </button>
-        <button
-          className="mx_Dialog_primary"
-          data-test-id="dialog-primary-button"
-          onClick={[Function]}
-          type="button"
-        >
-          Export
-        </button>
+          <button
+            data-test-id="dialog-cancel-button"
+            disabled={false}
+            onClick={[Function]}
+            type="button"
+          >
+            Cancel
+          </button>
+          <button
+            className="mx_Dialog_primary"
+            data-test-id="dialog-primary-button"
+            onClick={[Function]}
+            type="button"
+          >
+            Export
+          </button>
+        </span>
       </div>
     </DialogButtons>
   </div>,