From 040d1b70d48565115797d190f1735074c7911d83 Mon Sep 17 00:00:00 2001
From: Emma Segal-Grossman <hi@emma.cafe>
Date: Tue, 21 May 2024 13:28:41 -0400
Subject: [PATCH] Custom button cleanup & toolbar button styles (#1794)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes https://github.com/webrecorder/browsertrix/issues/1791



https://github.com/webrecorder/browsertrix/assets/5727389/54a4d9a2-9e68-4cbd-9cef-8a73a6106043

| Screenshots |
| --- |
| <img width="145" alt="Screenshot 2024-05-08 at 6 13 53 PM"
src="https://github.com/webrecorder/browsertrix/assets/5727389/35efd4b5-2a59-4fd2-8a62-904eedad98f9">
|
---
 frontend/src/components/ui/button.ts          | 52 +++++++++++++------
 frontend/src/components/ui/copy-button.ts     | 14 +++--
 frontend/src/components/ui/copy-field.ts      | 35 +++++++------
 .../crawl-workflows/queue-exclusion-form.ts   |  5 +-
 .../org/archived-item-qa/archived-item-qa.ts  |  9 ++--
 frontend/src/pages/org/collection-detail.ts   | 14 ++---
 6 files changed, 80 insertions(+), 49 deletions(-)

diff --git a/frontend/src/components/ui/button.ts b/frontend/src/components/ui/button.ts
index 2537548bb1..5f47e54d33 100644
--- a/frontend/src/components/ui/button.ts
+++ b/frontend/src/components/ui/button.ts
@@ -1,5 +1,6 @@
 /* eslint-disable lit/binding-positions */
 /* eslint-disable lit/no-invalid-html */
+import clsx from "clsx";
 import { css } from "lit";
 import { customElement, property } from "lit/decorators.js";
 import { ifDefined } from "lit/directives/if-defined.js";
@@ -8,6 +9,8 @@ import { html, literal } from "lit/static-html.js";
 import { TailwindElement } from "@/classes/TailwindElement";
 import { tw } from "@/utils/tailwind";
 
+type Variant = "neutral" | "danger";
+
 /**
  * Custom styled button
  *
@@ -21,8 +24,18 @@ export class Button extends TailwindElement {
   @property({ type: String })
   type: "submit" | "button" = "button";
 
+  // TODO unify button styling & variants - there are probably a few different
+  // approaches for difference button use cases, but we'll figure that out in
+  // the future when we work more on our UI library.
+  // See also https://github.com/webrecorder/browsertrix/issues/1550
   @property({ type: String })
-  variant: "primary" | "danger" | "neutral" = "neutral";
+  variant: Variant = "neutral";
+
+  @property({ type: Boolean })
+  filled = false;
+
+  @property({ type: String })
+  size: "small" | "medium" = "medium";
 
   @property({ type: String })
   label?: string;
@@ -39,9 +52,6 @@ export class Button extends TailwindElement {
   @property({ type: Boolean })
   loading = false;
 
-  @property({ type: Boolean })
-  icon = false;
-
   static styles = css`
     :host {
       display: inline-block;
@@ -57,18 +67,28 @@ export class Button extends TailwindElement {
     const tag = this.href ? literal`a` : literal`button`;
     return html`<${tag}
       type=${this.type === "submit" ? "submit" : "button"}
-      class=${[
-        tw`flex h-6 cursor-pointer items-center justify-center gap-2 rounded-sm text-center font-medium transition-all disabled:cursor-not-allowed disabled:text-neutral-300`,
-        this.icon ? tw`min-h-8 min-w-8 px-1` : tw`h-6 px-2`,
-        this.raised ? tw`shadow-sm` : "",
-        {
-          primary: tw`bg-blue-50 text-blue-600 shadow-blue-800/20 hover:bg-blue-100`,
-          danger: tw`shadow-danger-800/20 bg-danger-50 text-danger-600 hover:bg-danger-100`,
-          neutral: tw`text-neutral-600 hover:text-blue-600`,
-        }[this.variant],
-      ]
-        .filter(Boolean)
-        .join(" ")}
+      class=${clsx(
+        tw`flex h-6 cursor-pointer items-center justify-center gap-2 text-center font-medium outline-3 outline-offset-1 outline-primary transition focus-visible:outline disabled:cursor-not-allowed disabled:text-neutral-300`,
+        this.size === "medium"
+          ? tw`min-h-8 min-w-8 rounded-sm`
+          : tw`min-h-6 min-w-6 rounded-md`,
+        this.raised && tw`border shadow-sm`,
+        this.filled
+          ? [
+              tw`text-white`,
+              {
+                neutral: tw`border-primary-800 bg-primary-500 shadow-primary-800/20 hover:bg-primary-600`,
+                danger: tw`shadow-danger-800/20 border-danger-800 bg-danger-500 hover:bg-danger-600`,
+              }[this.variant],
+            ]
+          : [
+              this.raised && tw`bg-white`,
+              {
+                neutral: tw`border-gray-300 text-gray-600 hover:text-primary-600`,
+                danger: tw`shadow-danger-800/20 border-danger-300 bg-danger-50 text-danger-600 hover:bg-danger-100`,
+              }[this.variant],
+            ],
+      )}
       ?disabled=${this.disabled}
       href=${ifDefined(this.href)}
       aria-label=${ifDefined(this.label)}
diff --git a/frontend/src/components/ui/copy-button.ts b/frontend/src/components/ui/copy-button.ts
index 93aad09268..b8c363dce9 100644
--- a/frontend/src/components/ui/copy-button.ts
+++ b/frontend/src/components/ui/copy-button.ts
@@ -60,12 +60,18 @@ export class CopyButton extends LitElement {
         @sl-hide=${this.stopProp}
         @sl-after-hide=${this.stopProp}
       >
-        <sl-icon-button
-          name=${this.isCopied ? "check-lg" : this.name ? this.name : "copy"}
-          label=${msg("Copy to clipboard")}
+        <btrix-button
+          size="small"
           @click=${this.onClick}
           ?disabled=${!this.value && !this.getValue}
-        ></sl-icon-button>
+          class="inline"
+          raised
+        >
+          <sl-icon
+            name=${this.isCopied ? "check-lg" : this.name ? this.name : "copy"}
+            label=${msg("Copy to clipboard")}
+          ></sl-icon>
+        </btrix-button>
       </sl-tooltip>
     `;
   }
diff --git a/frontend/src/components/ui/copy-field.ts b/frontend/src/components/ui/copy-field.ts
index 7ed968d3b9..8b738b2e48 100644
--- a/frontend/src/components/ui/copy-field.ts
+++ b/frontend/src/components/ui/copy-field.ts
@@ -1,9 +1,10 @@
 import { localized } from "@lit/localize";
+import clsx from "clsx";
 import { css, html } from "lit";
-import { customElement, property } from "lit/decorators.js";
-import { classMap } from "lit/directives/class-map.js";
+import { customElement, property, queryAssignedNodes } from "lit/decorators.js";
 
 import { TailwindElement } from "@/classes/TailwindElement";
+import { tw } from "@/utils/tailwind";
 
 /**
  * Copy text to clipboard on click
@@ -49,26 +50,27 @@ export class CopyField extends TailwindElement {
     }
   `;
 
-  get _slottedChildren() {
-    const slot = this.shadowRoot?.querySelector("slot[name=label]");
-    return (slot as HTMLSlotElement | null | undefined)?.assignedElements();
-  }
+  @queryAssignedNodes({ slot: "label" })
+  private readonly slottedChildren: Element[] | undefined;
 
   render() {
     return html`
-      <div role="group">
+      <div
+        role="group"
+        class=${clsx(
+          tw`rounded border`,
+          this.filled && tw`bg-slate-50`,
+          this.monostyle && tw`font-monostyle`,
+        )}
+      >
         <label
-          class="${classMap({
-            hidden: !this.label && !this._slottedChildren,
-          })} mb-1.5 inline-block font-sans text-xs leading-[1.4] text-neutral-800"
+          class="${clsx(
+            "mb-1.5 inline-block font-sans text-xs leading-[1.4] text-neutral-800",
+            !this.label && !this.slottedChildren?.length && tw`hidden`,
+          )} "
           ><slot name="label">${this.label}</slot></label
         >
-        <div
-          class="${classMap({
-            "bg-slate-50": this.filled,
-            "font-monostyle": this.monostyle,
-          })} relative inline-flex w-full items-stretch justify-start rounded border"
-        >
+        <div class="relative inline-flex w-full items-stretch justify-start">
           <slot name="prefix"></slot>
           <span
             aria-hidden=${this.hideContentFromScreenReaders}
@@ -82,6 +84,7 @@ export class CopyField extends TailwindElement {
             .content=${this.buttonContent}
             .getValue=${this.getValue}
             .hoist=${this.hoist}
+            class="m-1 flex"
           ></btrix-copy-button>
         </div>
       </div>
diff --git a/frontend/src/features/crawl-workflows/queue-exclusion-form.ts b/frontend/src/features/crawl-workflows/queue-exclusion-form.ts
index bd396aa94d..29fbab2ee7 100644
--- a/frontend/src/features/crawl-workflows/queue-exclusion-form.ts
+++ b/frontend/src/features/crawl-workflows/queue-exclusion-form.ts
@@ -147,11 +147,10 @@ export class QueueExclusionForm extends LiteElement {
                   : ""}
               </sl-input>
             </div>
-            <div class="flex-0 w-10 pt-1 text-center">
+            <div class="flex-0 w-10 text-center">
               <btrix-button
-                variant="primary"
+                variant="neutral"
                 raised
-                icon
                 ?disabled=${!this.inputValue ||
                 this.isRegexInvalid ||
                 this.isSubmitting}
diff --git a/frontend/src/pages/org/archived-item-qa/archived-item-qa.ts b/frontend/src/pages/org/archived-item-qa/archived-item-qa.ts
index fa6958b714..4df2f32e5f 100644
--- a/frontend/src/pages/org/archived-item-qa/archived-item-qa.ts
+++ b/frontend/src/pages/org/archived-item-qa/archived-item-qa.ts
@@ -872,11 +872,14 @@ export class ArchivedItemQA extends TailwindElement {
                 placement="bottom-start"
               >
                 <btrix-button
-                  icon
-                  variant=${!this.splitView ? "primary" : "neutral"}
+                  raised
+                  ?filled=${!this.splitView}
+                  size="small"
                   @click="${() => (this.splitView = !this.splitView)}"
+                  class="m-0.5"
+                  aria-pressed=${!this.splitView}
                 >
-                  <sl-icon name="vr" label=${msg("Split view")}></sl-icon>
+                  <sl-icon name="vr"></sl-icon>
                 </btrix-button>
               </sl-tooltip>
             </div>
diff --git a/frontend/src/pages/org/collection-detail.ts b/frontend/src/pages/org/collection-detail.ts
index 21222d29c6..2dbb486470 100644
--- a/frontend/src/pages/org/collection-detail.ts
+++ b/frontend/src/pages/org/collection-detail.ts
@@ -300,12 +300,14 @@ export class CollectionDetail extends LiteElement {
           class="mb-2"
           .value="${publicReplayUrl}"
           hideContentFromScreenReaders
+          hoist
         >
-          <sl-tooltip slot="prefix" content=${msg("Open in New Tab")}>
+          <sl-tooltip slot="prefix" content=${msg("Open in New Tab")} hoist>
             <sl-icon-button
               href=${publicReplayUrl}
               name="box-arrow-up-right"
               target="_blank"
+              class="m-px"
             >
             </sl-icon-button>
           </sl-tooltip>
@@ -323,12 +325,11 @@ export class CollectionDetail extends LiteElement {
         </p>
         <div class="relative mb-5 rounded border bg-slate-50 p-3 pr-9">
           <btrix-code value=${embedCode}></btrix-code>
-          <div
-            class="absolute right-1.5 top-1.5 rounded border bg-white shadow-sm"
-          >
+          <div class="absolute right-1 top-1">
             <btrix-copy-button
               .getValue=${() => embedCode}
               content=${msg("Copy Embed Code")}
+              hoist
             ></btrix-copy-button>
           </div>
         </div>
@@ -340,12 +341,11 @@ export class CollectionDetail extends LiteElement {
         </p>
         <div class="relative mb-5 rounded border bg-slate-50 p-3 pr-9">
           <btrix-code language="javascript" value=${importCode}></btrix-code>
-          <div
-            class="absolute right-1.5 top-1.5 rounded border bg-white shadow-sm"
-          >
+          <div class="absolute right-1 top-1">
             <btrix-copy-button
               .getValue=${() => importCode}
               content=${msg("Copy JS")}
+              hoist
             ></btrix-copy-button>
           </div>
         </div>