feat: button styles and button-group #54

arnest00 · 2025-04-24T04:35:11Z

Summary of changes

Remove automatic link styling as buttons
Create button styles
Create button-group block

Relevant Links

Designs: Figma
Stories: ADB-163, ADB-164

Test URLs:

Checklist

This PR has visual changes, and has been reviewed by a designer.
This PR has code changes, and our linters still pass.
This PR has new code, so new tests were added or updated, and they pass.
This PR affects production code, so it was browser tested (see below).
The content of this PR requires documentation, so we added a detailed description of the component's purpose, requirements, quirks, and instructions for use by designers and developers. This includes accessibility information if pertinent.

Validation

Visit the branch preview page
Scroll down to the Button-Group section
Verify the Button-Group is responsive (flex-direction: column on mobile screens, flex-direction: row on screens larger than tablet)
Verify the Buttons within the group:
a. are <a> elements with button classes applied
b. have default, hover, and focus states
Open the dev console and enter the following:

const buttonGroup = document.querySelector('div.button-group');
const newButton = document.createElement('button');
newButton.innerText = "Test Button";
newButton.classList = "button button--primary";
buttonGroup.append(newButton);

Change the class of this button from button--primary to button--primary-outline, button--accent, button--static-black, button--static-white, and button--ghost through the dev console to see how the button styles look on <button> elements
Force the button into a disabled state for each button style to verify inclusion
Switch themes (from light mode to dark mode, or vice versa) and repeat steps 4, 6, and 7

Browser Testing

We should aim to support the latest version of the listed browsers. For older versions or other browsers not on the list, content should be accessible, even if it doesn't completely match the designs.

Developers should test as they work in the browsers available on their machines. If they have access to other devices to test other browser/OS combinations, they should do that when possible.

Windows

Firefox
Chrome
Edge

MacOS

Firefox
Chrome
Safari
Edge

Android

Firefox
Chrome
Edge

iOS

Safari

aem-code-sync · 2025-04-24T04:35:15Z

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

Re-run PSI checks
Re-sync branch

Commits

c801b7b ✅ (latest)
7a8a7bb ✅
e70d468 ✅
012d948 ✅
18af2f6 ✅
ba7f92e ✅
24d57b2 ✅

aem-code-sync · 2025-04-24T04:35:40Z

	Page	Scores	Audits	Google
📱	/pattern-library/
🖥️	/pattern-library/

catheraaine

Button group is great, it solves for a single button placed as a block, too, like we might need in the jobs pre-footer. I am sure you're not yet finished with documentation, but could you add a note in there that a button group can include just one button?

Some notes:

1. 😏 `-webkit-font-smoothing: antialiased;`

2. I think we're using the wrong blues? Visually they look different to me on both default and hover

3. We'll need the static white button for over callouts, might as well build the static black one, too

catheraaine · 2025-04-24T13:25:17Z

blocks/button-group/button-group.js

+    anchorNode: row.children[0].firstChild.firstChild,
+    buttonStyle: row.children[1].firstChild,
+    altText: row.children[2].firstChild
+  }));


I really like this map solve for making the components and I've stolen it for my own work.

catheraaine · 2025-04-24T13:26:24Z

scripts/aem.js

-    }
-  });
-}
-


catheraaine · 2025-04-24T13:28:28Z

styles/styles.css

-  background-color: var(--color-background-button-hover);
-  cursor: pointer;
+.button--primary {
+  --color-button-primary-background: rgb(from var(--spectrum-black) r g b / 84%);


Suggested change

--color-button-primary-background: rgb(from var(--spectrum-black) r g b / 84%);

--color-button-background-primary: rgb(from var(--spectrum-black) r g b / 84%);

catheraaine · 2025-04-24T13:29:25Z

styles/styles.css

+  --color-button-primary-background: rgb(from var(--spectrum-black) r g b / 84%);
+  --color-button-primary-background-hover: rgb(from var(--spectrum-black) r g b / 93%);
+  --color-button-primary-background-disabled: rgb(from var(--spectrum-black) r g b / 9%);
+  --color-text-button-primary: var(--spectrum-white);


Suggested change

--color-text-button-primary: var(--spectrum-white);

--color-button-text-primary: var(--spectrum-white);

or

--color-background-button-primary:

since these have been moved out of global scope and into component scope, we should have the component first, methinks

Good point! I suppose if I was going to organize this in Figma or something, it'd go

--color-button-background-primary: /* ... */ --color-button-background-primary-hover: /* ... */

and so on.

catheraaine · 2025-04-24T13:30:34Z

styles/styles.css

+  --color-button-primary-background: rgb(from var(--spectrum-white) r g b / 85%);
+  --color-button-primary-background-hover: rgb(from var(--spectrum-white) r g b / 94%);
+  --color-button-primary-background-disabled: rgb(from var(--spectrum-white) r g b / 11%);
+  --color-text-button-primary: var(--spectrum-gray-25);


i'm not going to make a million comments about cleaning up your variable names, I think you probably get it.

catheraaine · 2025-04-24T13:35:23Z

styles/styles.css

+  }
+}
+
+.button--outline {


Can we rename the button themes to match Spectrum?
https://opensource.adobe.com/spectrum-css/?path=/docs/components-button--docs

Accent, Primary, Static Black, Static White

With the Primary having Filled and Outline variants, they should now be as follows:

primary

primary-outline

accent

static-black (added)

static-white (added)

ghost

styles/styles.css

arnest00 · 2025-04-25T14:30:21Z

styles/base.css

  --spectrum-blue-500: rgb(142 185 252);
  --spectrum-blue-800: rgb(75 117 255);
  --spectrum-blue-900: rgb(59 99 251);
+  --spectrum-blue-1000: rgb(39, 77, 234);


Added a missing color from S2 feature branch

styles/styles.css

catheraaine

Superb work, and the documentation looks great, too!

arnest00 added the WIP Work in progress, likely a draft. label Apr 24, 2025

arnest00 force-pushed the feat-button-group branch from 0591bad to 36855f0 Compare April 24, 2025 04:35

aem-code-sync bot temporarily deployed to feat-button-group April 24, 2025 04:35 Inactive

arnest00 force-pushed the feat-button-group branch from 36855f0 to 836cc5d Compare April 24, 2025 05:32

aem-code-sync bot temporarily deployed to feat-button-group April 24, 2025 05:32 Inactive

arnest00 force-pushed the feat-button-group branch from 836cc5d to c96456c Compare April 24, 2025 05:41

aem-code-sync bot temporarily deployed to feat-button-group April 24, 2025 05:41 Inactive

arnest00 changed the title ~~wip: add button styles, create button-group block~~ feat: add button styles, create button-group block Apr 24, 2025

arnest00 marked this pull request as ready for review April 24, 2025 05:45

arnest00 added Ready for Dev Review and removed WIP Work in progress, likely a draft. labels Apr 24, 2025

catheraaine requested changes Apr 24, 2025

View reviewed changes

catheraaine added Changes Requested and removed Ready for Dev Review labels Apr 24, 2025

catheraaine changed the title ~~feat: add button styles, create button-group block~~ feat: button styles and button-group Apr 24, 2025

aem-code-sync bot temporarily deployed to feat-button-group April 25, 2025 14:29 Inactive

arnest00 commented Apr 25, 2025

View reviewed changes

styles/styles.css Show resolved Hide resolved

arnest00 commented Apr 25, 2025

View reviewed changes

styles/styles.css Show resolved Hide resolved

aem-code-sync bot temporarily deployed to feat-button-group April 25, 2025 14:40 Inactive

catheraaine added Ready for Dev Review and removed Changes Requested labels Apr 25, 2025

aem-code-sync bot temporarily deployed to feat-button-group April 25, 2025 16:26 Inactive

arnest00 added 5 commits April 25, 2025 09:58

feat: remove automatic link styling as buttons

24d57b2

feat: remove button styles from base.css

ba7f92e

feat: create button styles

18af2f6

feat: create button-group block

012d948

fix: address feedback

e70d468

arnest00 added 2 commits April 25, 2025 09:58

fix: link colors and ghost button styles

7a8a7bb

fix: css value formatting

c801b7b

arnest00 force-pushed the feat-button-group branch from 5e7e2e6 to c801b7b Compare April 25, 2025 17:15

aem-code-sync bot deployed to feat-button-group April 25, 2025 17:15 View deployment

catheraaine approved these changes Apr 25, 2025

View reviewed changes

catheraaine added Ready for Squash & Merge and removed Ready for Dev Review labels Apr 25, 2025

arnest00 merged commit 2efb9f2 into main Apr 25, 2025
5 of 7 checks passed

	--color-button-primary-background: rgb(from var(--spectrum-black) r g b / 84%);
	--color-button-background-primary: rgb(from var(--spectrum-black) r g b / 84%);

	--color-text-button-primary: var(--spectrum-white);
	--color-button-text-primary: var(--spectrum-white);

feat: button styles and button-group #54

feat: button styles and button-group #54

Uh oh!

Conversation

arnest00 commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Relevant Links

Test URLs:

Checklist

Validation

Browser Testing

Uh oh!

aem-code-sync bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aem-code-sync bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

catheraaine left a comment

Choose a reason for hiding this comment

1. 😏 -webkit-font-smoothing: antialiased;

2. I think we're using the wrong blues? Visually they look different to me on both default and hover

3. We'll need the static white button for over callouts, might as well build the static black one, too

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

catheraaine left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arnest00 commented Apr 24, 2025 •

edited

Loading

aem-code-sync bot commented Apr 24, 2025 •

edited

Loading

aem-code-sync bot commented Apr 24, 2025 •

edited

Loading

1. 😏 `-webkit-font-smoothing: antialiased;`