From 60514dd8e61749ae11825c02a5ff37782016fc55 Mon Sep 17 00:00:00 2001
From: Florian Scholdei <florian.scholdei@cloudogu.com>
Date: Tue, 11 Mar 2025 11:37:44 +0100
Subject: [PATCH] Improve usability and accessibility of tertiary button

Squash commits of branch feature/tertiary_buttons:

- Improve usability and accessibility of tertiary button

- Remove special cases for outline variants
---
 gradle/changelog/tertiary_buttons.yaml        |   2 +
 .../src/base/buttons/Button.test.stories.mdx  | 143 ++++++++++--------
 scm-ui/ui-styles/src/components/_main.scss    | 123 ++++++---------
 scm-ui/ui-styles/src/highcontrast.scss        | 120 ++++-----------
 scm-ui/ui-styles/src/light.scss               |  13 --
 5 files changed, 158 insertions(+), 243 deletions(-)
 create mode 100644 gradle/changelog/tertiary_buttons.yaml

diff --git a/gradle/changelog/tertiary_buttons.yaml b/gradle/changelog/tertiary_buttons.yaml
new file mode 100644
index 0000000000..b17d5a6f7e
--- /dev/null
+++ b/gradle/changelog/tertiary_buttons.yaml
@@ -0,0 +1,2 @@
+- type: fixed
+  description: Usability and accessibility of tertiary button
diff --git a/scm-ui/ui-core/src/base/buttons/Button.test.stories.mdx b/scm-ui/ui-core/src/base/buttons/Button.test.stories.mdx
index 0a6db5dccd..ac352ca251 100644
--- a/scm-ui/ui-core/src/base/buttons/Button.test.stories.mdx
+++ b/scm-ui/ui-core/src/base/buttons/Button.test.stories.mdx
@@ -1,74 +1,87 @@
 import { Meta, Story } from "@storybook/addon-docs";
 import { Button, ButtonVariantList } from "./Button";
 
-<Meta title="Tests"/>
+<Meta title="Buttons" />
 
-<Story name="Light States" parameters={{
-  pseudo: {
-    hover: ButtonVariantList.map(variant => `#${variant}-Hover`),
-    focus: ButtonVariantList.map(variant => `#${variant}-Focus`),
-    active: ButtonVariantList.map(variant => `#${variant}-Active`),
-  },
-  themes: {
-    default: 'light',
-  },
-}}>
-  <table>
-    <tr>
-      <th>STATE</th>
-      {ButtonVariantList.map(variant => <th>{variant.toUpperCase()}</th>)}
-    </tr>
-    {["Normal", "Hover", "Active", "Focus", "Disabled", "Loading"].map(state => <tr>
-      <td>{state}</td>
-      {ButtonVariantList.map(variant => <td><Button id={`${variant}-${state}`} disabled={state === "Disabled"}
-                                                    isLoading={state === "Loading"}
-                                                    variant={variant}>Button</Button></td>)}
-    </tr>)}
-  </table>
+export const renderTableContent = () => (
+  <>
+    {["Normal", "Hover", "Focus", "Active", "Disabled", "Loading"].map((state) => (
+      <tr>
+        <td>{state}</td>
+        {ButtonVariantList.map((variant) => (
+          <td>
+            <Button
+              id={`${variant}-${state}`}
+              disabled={state === "Disabled"}
+              isLoading={state === "Loading"}
+              className={
+                state === "Hover"
+                  ? "is-hovered"
+                  : state === "Focus"
+                  ? "is-focused"
+                  : state === "Active"
+                  ? "is-active"
+                  : ""
+              }
+              variant={variant}
+            >
+              Button
+            </Button>
+          </td>
+        ))}
+      </tr>
+    ))}
+  </>
+);
+
+export const ButtonStates = (args) => {
+  return (
+    <>
+      <table className="table is-narrow">
+        <tr>
+          <th>STATE</th>
+          {ButtonVariantList.map((variant) => (
+            <th>{variant.toUpperCase()}</th>
+          ))}
+        </tr>
+        {renderTableContent()}
+      </table>
+      <table className="table is-narrow has-background-dark has-text-white">
+         {renderTableContent()}
+      </table>
+    </>
+  );
+};
+
+<Story
+  name="Light States"
+  parameters={{
+    themes: {
+      default: "light",
+    },
+  }}
+>
+  <ButtonStates />
 </Story>
 
-<Story name="Dark States" parameters={{
-  pseudo: {
-    hover: ButtonVariantList.map(variant => `#${variant}-Hover`),
-    focus: ButtonVariantList.map(variant => `#${variant}-Focus`),
-    active: ButtonVariantList.map(variant => `#${variant}-Active`),
-  },
-  themes: {
-    default: 'dark',
-  },
-}}>
-  <table>
-    <tr>
-      <th>STATE</th>
-      {ButtonVariantList.map(variant => <th>{variant.toUpperCase()}</th>)}
-    </tr>
-    {["Normal", "Hover", "Active", "Focus", "Disabled"].map(state => <tr>
-      <td>{state}</td>
-      {ButtonVariantList.map(variant => <td><Button id={`${variant}-${state}`} disabled={state === "Disabled"}
-                                                    variant={variant}>Button</Button></td>)}
-    </tr>)}
-  </table>
+<Story
+  name="Dark States"
+  parameters={{
+    themes: {
+      default: "dark",
+    },
+  }}
+>
+  <ButtonStates />
 </Story>
 
-<Story name="High-Contrast States" parameters={{
-  pseudo: {
-    hover: ButtonVariantList.map(variant => `#${variant}-Hover`),
-    focus: ButtonVariantList.map(variant => `#${variant}-Focus`),
-    active: ButtonVariantList.map(variant => `#${variant}-Active`),
-  },
-  themes: {
-    default: 'highcontrast',
-  },
-}}>
-  <table>
-    <tr>
-      <th>STATE</th>
-      {ButtonVariantList.map(variant => <th>{variant.toUpperCase()}</th>)}
-    </tr>
-    {["Normal", "Hover", "Active", "Focus", "Disabled"].map(state => <tr>
-      <td>{state}</td>
-      {ButtonVariantList.map(variant => <td><Button id={`${variant}-${state}`} disabled={state === "Disabled"}
-                                                    variant={variant}>Button</Button></td>)}
-    </tr>)}
-  </table>
+<Story
+  name="High-Contrast States"
+  parameters={{
+    themes: {
+      default: "highcontrast",
+    },
+  }}
+>
+  <ButtonStates />
 </Story>
diff --git a/scm-ui/ui-styles/src/components/_main.scss b/scm-ui/ui-styles/src/components/_main.scss
index 7c2712c03c..e22c13eec8 100644
--- a/scm-ui/ui-styles/src/components/_main.scss
+++ b/scm-ui/ui-styles/src/components/_main.scss
@@ -364,7 +364,8 @@ a {
   }
 }
 
-button, .button {
+button,
+.button {
   &:focus-visible,
   &.is-focused {
     outline: $focus-outline-color $focus-outline-style;
@@ -385,6 +386,20 @@ summary {
 }
 
 // buttons
+@mixin button-variant($name, $variant, $variant-invert: $scheme-main) {
+  &.is-#{$name} {
+    &:hover,
+    &.is-hovered {
+      background-color: scale-color($variant, $lightness: -10%);
+    }
+    &:active,
+    &.is-active {
+      color: $variant-invert;
+      background-color: scale-color($variant, $lightness: -20%);
+    }
+  }
+}
+
 .button {
   padding-left: 1.5em;
   padding-right: 1.5em;
@@ -392,75 +407,37 @@ summary {
   min-width: 2.5rem;
   font-weight: $weight-semibold;
 
-  &.is-primary,
-  &.is-info,
-  &.is-link,
-  &.is-success,
-  &.is-warning,
-  &.is-danger {
-    &.is-outlined {
-      background-color: inherit;
+  @include button-variant("primary", $primary, $primary-invert);
+  @include button-variant("info", $info, $info-invert);
+  @include button-variant("link", $link, $link-invert);
+  @include button-variant("success", $success, $success-invert);
+  @include button-variant("warning", $warning, $warning-invert);
+  @include button-variant("danger", $danger, $danger-invert);
+
+  &.is-inverted.is-primary {
+    &:hover,
+    &.is-hovered {
+      background-color: transparent;
+      border-color: $primary;
     }
-  }
-
-  &.is-primary:hover,
-  &.is-primary.is-hovered {
-    background-color: scale-color($primary, $lightness: -10%);
-  }
-
-  &.is-primary:active,
-  &.is-primary.is-active {
-    background-color: scale-color($primary, $lightness: -20%);
-  }
-
-  &.is-info:hover,
-  &.is-info.is-hovered {
-    background-color: scale-color($info, $lightness: -10%);
-  }
-
-  &.is-info:active,
-  &.is-info.is-active {
-    background-color: scale-color($info, $lightness: -20%);
-  }
-
-  &.is-link:hover,
-  &.is-link.is-hovered {
-    background-color: scale-color($link, $lightness: -10%);
-  }
-
-  &.is-link:active,
-  &.is-link.is-active {
-    background-color: scale-color($link, $lightness: -20%);
-  }
-
-  &.is-success:hover,
-  &.is-success.is-hovered {
-    background-color: scale-color($success, $lightness: -10%);
-  }
-
-  &.is-success:active,
-  &.is-success.is-active {
-    background-color: scale-color($success, $lightness: -20%);
-  }
-
-  &.is-warning:hover,
-  &.is-warning.is-hovered {
-    background-color: scale-color($warning, $lightness: -10%);
-  }
-
-  &.is-warning:active,
-  &.is-warning.is-active {
-    background-color: scale-color($warning, $lightness: -20%);
-  }
-
-  &.is-danger:hover,
-  &.is-danger.is-hovered {
-    background-color: scale-color($danger, $lightness: -10%);
-  }
-
-  &.is-danger:active,
-  &.is-danger.is-active {
-    background-color: scale-color($danger, $lightness: -20%);
+    &:focus,
+    &.is-focused {
+      border-color: $primary;
+    }
+    &:active,
+    &.is-active {
+      color: $primary-invert;
+      background-color: $primary;
+    }
+    &[disabled] {
+      color: $primary;
+      background-color: transparent;
+      border-color: transparent;
+    }
+    &.is-loading::after {
+      border-color: transparent transparent $primary $primary !important;
+    }
+    background-color: transparent;
   }
 
   &.is-reduced-mobile,
@@ -806,7 +783,7 @@ form .field:not(.is-grouped) {
 }
 
 .has-scm-background {
-  background-image: url(images/scmManagerHero.jpg) !important;
+  background-image: url("images/scmManagerHero.jpg") !important;
   background-size: cover;
   background-position: top center;
   background-color: #002e4b;
@@ -815,7 +792,7 @@ form .field:not(.is-grouped) {
 // hero
 .hero.is-dark {
   background-color: #002e4b;
-  background-image: url(images/scmManagerHero.jpg);
+  background-image: url("images/scmManagerHero.jpg");
   background-size: cover;
   background-position: top center;
 
@@ -1086,7 +1063,7 @@ form .field:not(.is-grouped) {
 
 .space_char::before {
   font-size: 0.75rem;
-  content: '·';
+  content: "·";
   color: var(--diff-whitespace-color);
   background: transparent;
   position: absolute;
@@ -1094,7 +1071,7 @@ form .field:not(.is-grouped) {
 
 .tabulator_char::before {
   font-size: 0.75rem;
-  content: '↦';
+  content: "↦";
   color: var(--diff-whitespace-color);
   background: transparent;
   position: absolute;
diff --git a/scm-ui/ui-styles/src/highcontrast.scss b/scm-ui/ui-styles/src/highcontrast.scss
index 38623e9cda..30d103e204 100644
--- a/scm-ui/ui-styles/src/highcontrast.scss
+++ b/scm-ui/ui-styles/src/highcontrast.scss
@@ -59,7 +59,7 @@ $box-background-color: $grey-darker;
 $table-row-hover-background-color: $grey-darker;
 
 /* Focus for Buttons */
-$focus-outline-color: #C777EE;
+$focus-outline-color: #c777ee;
 
 /* Components */
 $menu-label-color: $scheme-main;
@@ -125,94 +125,32 @@ $tooltip-color: $scheme-main;
   --diff-whitespace-color: #fffce0;
 }
 
-.button {
-  &.is-primary:hover,
-  &.is-primary.is-hovered {
-    background-color: scale-color($primary, $lightness: -5%);
-  }
-
-  &.is-primary:active,
-  &.is-primary.is-active {
-    background-color: scale-color($primary, $lightness: -10%);
-  }
-
-  &.is-primary[disabled] {
-    background-color: scale-color($primary, $lightness: -50%);
-  }
-
-  &.is-info:hover,
-  &.is-info.is-hovered {
-    background-color: scale-color($info, $lightness: -5%);
-  }
-
-  &.is-info:active,
-  &.is-info.is-active {
-    background-color: scale-color($info, $lightness: -10%);
-  }
-
-  &.is-info[disabled] {
-    background-color: scale-color($info, $lightness: -50%);
-  }
-
-  &.is-link:hover,
-  &.is-link.is-hovered {
-    background-color: scale-color($link, $lightness: -5%);
-  }
-
-  &.is-link:active,
-  &.is-link.is-active {
-    background-color: scale-color($link, $lightness: -10%);
-  }
-
-  &.is-link[disabled] {
-    background-color: scale-color($link, $lightness: -50%);
-  }
-
-  &.is-success:hover,
-  &.is-success.is-hovered {
-    background-color: scale-color($success, $lightness: -5%);
-  }
-
-  &.is-success:active,
-  &.is-success.is-active {
-    background-color: scale-color($success, $lightness: -10%);
-  }
-
-  &.is-success[disabled] {
-    background-color: scale-color($success, $lightness: -50%);
-  }
-
-  &.is-warning:hover,
-  &.is-warning.is-hovered {
-    background-color: scale-color($warning, $lightness: -5%);
-  }
-
-  &.is-warning:active,
-  &.is-warning.is-active {
-    background-color: scale-color($warning, $lightness: -10%);
-  }
-
-  &.is-warning[disabled] {
-    background-color: scale-color($warning, $lightness: -50%);
-    color: #e1d4c2;
-  }
-
-  &.is-danger:hover,
-  &.is-danger.is-hovered {
-    background-color: scale-color($danger, $lightness: -5%);
-  }
-
-  &.is-danger:active,
-  &.is-danger.is-active {
-    background-color: scale-color($danger, $lightness: -10%);
-  }
-
-  &.is-danger[disabled] {
-    background-color: scale-color($danger, $lightness: -50%);
+@mixin button-variant($name, $variant, $variant-invert: $scheme-main) {
+  &.is-#{$name} {
+    &:hover,
+    &.is-hovered {
+      background-color: scale-color($variant, $lightness: -5%);
+    }
+    &:active,
+    &.is-active {
+      color: $variant-invert;
+      background-color: scale-color($variant, $lightness: -10%);
+    }
+    &:disabled {
+      background-color: scale-color($variant, $lightness: -50%);
+    }
   }
 }
 
-//footer is overwritten in _main.scss
+.button {
+  @include button-variant("primary", $primary, $primary-invert);
+  @include button-variant("info", $info, $info-invert);
+  @include button-variant("link", $link, $link-invert);
+  @include button-variant("success", $success, $success-invert);
+  @include button-variant("warning", $warning, $warning-invert);
+  @include button-variant("danger", $danger, $danger-invert);
+}
+
 footer.footer {
   a {
     color: scale-color($link, $lightness: 25%);
@@ -235,7 +173,6 @@ td button:not(.is-default) {
 //card
 .modal-card {
   border: 1px solid $border;
-
   & .modal-close,
   & .delete {
     background-color: $white-ter;
@@ -248,11 +185,9 @@ td button:not(.is-default) {
     a {
       color: $white-bis;
     }
-
     &:hover {
       td {
         background-color: $grey-darker;
-
         &.is-darker {
           background-color: $grey-darker;
         }
@@ -294,10 +229,11 @@ td button:not(.is-default) {
   color: $scheme-main;
 }
 
-.hunk-expander{
-  &:hover, :focus-visible {
+.hunk-expander {
+  &:hover,
+  :focus-visible {
     color: #98d8f3;
-    background-color: #050514
+    background-color: #050514;
   }
 }
 
diff --git a/scm-ui/ui-styles/src/light.scss b/scm-ui/ui-styles/src/light.scss
index 3b6ece8a37..416241fd66 100644
--- a/scm-ui/ui-styles/src/light.scss
+++ b/scm-ui/ui-styles/src/light.scss
@@ -75,19 +75,6 @@ $popover-border-color: $grey-lightest;
   --diff-whitespace-color: #363636;
 }
 
-.button {
-  &.is-primary,
-  &.is-info,
-  &.is-link,
-  &.is-success,
-  &.is-warning,
-  &.is-danger {
-    &.is-outlined {
-      background-color: $white;
-    }
-  }
-}
-
 footer.footer {
   a {
     color: darken($info, 15%);