Conversation
Contributor
Reviewer's GuideThis PR refactors aria-label support in the AuroButton component by introducing dedicated slots for default and loading state labels (deprecating the loadingText property), updates documentation and examples to use the new slot-based API, enhances tests to cover these changes, bumps the auro-library dependency, and removes an outdated demo script. Class diagram for updated AuroButton aria-label logicclassDiagram
class AuroButton {
+slot ariaLabel
+slot ariaLabel.loading
-loadingText : String (deprecated)
+generateAriaLabel() : String
-currentAriaLabel : String
-currentAriaLabelledBy : String
}
AuroButton --> "1" runtimeUtils : uses
class runtimeUtils {
+getSlotText(element, slotName) : String
+handleComponentTagRename(element, tagName)
}
Class diagram for deprecated loadingText propertyclassDiagram
class AuroButton {
-loadingText : String (deprecated)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider adding a console warning or fallback to catch buttons without any ariaLabel slot or attribute to prevent accidental accessibility regressions.
- The bulk update of example files could be streamlined by centralizing snippet templates or using a shared include mechanism to reduce future maintenance overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a console warning or fallback to catch buttons without any ariaLabel slot or attribute to prevent accidental accessibility regressions.
- The bulk update of example files could be streamlined by centralizing snippet templates or using a shared include mechanism to reduce future maintenance overhead.
## Individual Comments
### Comment 1
<location> `src/auro-button.js:29-35` </location>
<code_context>
/**
* @slot - Default slot for the text of the button.
+ * @slot ariaLabel - Use this slot to pass an aria-label to the HTML5 button.
+ * @slot ariaLabel.loading - Use this slot to pass an aria-label to the HTML5 button when in loading state.
* @csspart button - Apply CSS to HTML5 button.
* @csspart loader - Apply CSS to auro-loader.
</code_context>
<issue_to_address>
**suggestion:** Consider clarifying slot usage for aria-labels in documentation.
Specify in the documentation that only plain text should be used in these slots for aria-labels, without interactive elements or markup, to ensure proper accessibility.
```suggestion
/**
* @slot - Default slot for the text of the button.
* @slot ariaLabel - Use this slot to pass an aria-label (plain text only, no markup or interactive elements) to the HTML5 button for accessibility.
* @slot ariaLabel.loading - Use this slot to pass an aria-label (plain text only, no markup or interactive elements) to the HTML5 button when in loading state for accessibility.
* @csspart button - Apply CSS to HTML5 button.
* @csspart loader - Apply CSS to auro-loader.
* @csspart text - Apply CSS to text slot.
```
</issue_to_address>
### Comment 2
<location> `test/auro-button.test.js:137-149` </location>
<code_context>
expect(button.getAttribute("aria-label")).to.equal("label");
});
+ it("tests setting aria-label via slot", async () => {
+ const el = await fixture(html`
+ <auro-button>
+ <span slot="ariaLabel">label</span>
+ Click Me!
+ </auro-button>
+ `);
+
+ const root = el.shadowRoot;
+ const button = root.querySelector("button");
+
+ expect(button.getAttribute("aria-label")).to.equal("label");
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for multiple ariaLabel slots or conflicting sources.
Please add a test where both the aria-label attribute and ariaLabel slot are set, to confirm which value is used.
```suggestion
it("tests setting aria-label via slot", async () => {
const el = await fixture(html`
<auro-button>
<span slot="ariaLabel">label</span>
Click Me!
</auro-button>
`);
const root = el.shadowRoot;
const button = root.querySelector("button");
expect(button.getAttribute("aria-label")).to.equal("label");
});
it("tests aria-label precedence: slot vs attribute", async () => {
const el = await fixture(html`
<auro-button aria-label="attributeLabel">
<span slot="ariaLabel">slotLabel</span>
Click Me!
</auro-button>
`);
const root = el.shadowRoot;
const button = root.querySelector("button");
// Expect slot to take precedence over attribute
expect(button.getAttribute("aria-label")).to.equal("slotLabel");
});
```
</issue_to_address>
### Comment 3
<location> `test/auro-button.test.js:173-184` </location>
<code_context>
expect(button.getAttribute("aria-label")).to.equal(el.loadingText);
});
+ it("tests setting custom loading text on button in loading state via slot", async () => {
+ const el = await fixture(html`
+ <auro-button loading>
+ <span slot="ariaLabel.loading">Cargando...</span>
+ </auro-button>
+ `);
+
+ const root = el.shadowRoot;
+ const button = root.querySelector("button");
+
+ expect(button.getAttribute("aria-label")).to.equal("Cargando...");
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for fallback behavior when ariaLabel.loading slot is empty or missing.
Add a test to verify that when the ariaLabel.loading slot is empty or missing, the button correctly falls back to loadingText or the default 'Loading...'.
```suggestion
it("tests setting custom loading text on button in loading state via slot", async () => {
const el = await fixture(html`
<auro-button loading>
<span slot="ariaLabel.loading">Cargando...</span>
</auro-button>
`);
const root = el.shadowRoot;
const button = root.querySelector("button");
expect(button.getAttribute("aria-label")).to.equal("Cargando...");
});
it("falls back to loadingText when ariaLabel.loading slot is missing", async () => {
const el = await fixture(html`
<auro-button loading loadingText="Cargando">Click Me!</auro-button>
`);
const root = el.shadowRoot;
const button = root.querySelector("button");
expect(button.getAttribute("aria-label")).to.equal("Cargando");
});
it("falls back to loadingText when ariaLabel.loading slot is empty", async () => {
const el = await fixture(html`
<auro-button loading loadingText="Cargando">
<span slot="ariaLabel.loading"></span>
</auro-button>
`);
const root = el.shadowRoot;
const button = root.querySelector("button");
expect(button.getAttribute("aria-label")).to.equal("Cargando");
});
it("falls back to default 'Loading...' when ariaLabel.loading slot and loadingText are missing", async () => {
const el = await fixture(html`
<auro-button loading>Click Me!</auro-button>
`);
const root = el.shadowRoot;
const button = root.querySelector("button");
expect(button.getAttribute("aria-label")).to.equal("Loading...");
});
```
</issue_to_address>
### Comment 4
<location> `test/auro-button.test.js:156-160` </location>
<code_context>
+ expect(button.getAttribute("aria-label")).to.equal("label");
+ });
+
it("tests that button in loading state has proper aria-label", async () => {
const el = await fixture(html`
<auro-button loading>Click Me!</auro-button>
</code_context>
<issue_to_address>
**suggestion (testing):** Consider testing for dynamic changes to loading state and aria-label.
Add a test that toggles the loading state after render to confirm the aria-label updates as expected.
```suggestion
it("tests that button in loading state has proper aria-label", async () => {
const el = await fixture(html`
<auro-button loading>Click Me!</auro-button>
`);
const root = el.shadowRoot;
const button = root.querySelector("button");
expect(button.getAttribute("aria-label")).to.equal("Loading...");
});
it("tests dynamic changes to loading state update aria-label", async () => {
const el = await fixture(html`
<auro-button aria-label="label">Click Me!</auro-button>
`);
const root = el.shadowRoot;
const button = root.querySelector("button");
// Initial state
expect(button.getAttribute("aria-label")).to.equal("label");
// Toggle loading on
el.loading = true;
await el.updateComplete;
expect(button.getAttribute("aria-label")).to.equal("Loading...");
// Toggle loading off
el.loading = false;
await el.updateComplete;
expect(button.getAttribute("aria-label")).to.equal("label");
});
```
</issue_to_address>
### Comment 5
<location> `demo/api.md:496` </location>
<code_context>
-#### Right Aligned
+## Right Aligned
This example shows a `rounded` `auro-button` that is right-aligned, demonstrating how the button starts from the right and grows/shrinks from right to left when using the `toggleText` attribute in conjuction with the `mouseover` and `mouseout` events. The `focusin` and `focusout` events simulate toggling text for keyboard users.
</code_context>
<issue_to_address>
**issue (typo):** Typo: 'conjuction' should be 'conjunction'.
Update the sentence to use the correct spelling: 'conjunction'.
```suggestion
This example shows a `rounded` `auro-button` that is right-aligned, demonstrating how the button starts from the right and grows/shrinks from right to left when using the `toggleText` attribute in conjunction with the `mouseover` and `mouseout` events. The `focusin` and `focusout` events simulate toggling text for keyboard users.
```
</issue_to_address>
### Comment 6
<location> `docs/partials/api.md:186` </location>
<code_context>
-#### Right Aligned
+## Right Aligned
This example shows a `rounded` `auro-button` that is right-aligned, demonstrating how the button starts from the right and grows/shrinks from right to left when using the `toggleText` attribute in conjuction with the `mouseover` and `mouseout` events. The `focusin` and `focusout` events simulate toggling text for keyboard users.
</code_context>
<issue_to_address>
**issue (typo):** Typo: 'conjuction' should be 'conjunction'.
Please update the spelling to 'conjunction'.
```suggestion
This example shows a `rounded` `auro-button` that is right-aligned, demonstrating how the button starts from the right and grows/shrinks from right to left when using the `toggleText` attribute in conjunction with the `mouseover` and `mouseout` events. The `focusin` and `focusout` events simulate toggling text for keyboard users.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jordanjones243
approved these changes
Oct 9, 2025
|
🎉 This PR is included in version 12.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alaska Airlines Pull Request
Checklist:
By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team
Summary by Sourcery
Enable slot-based ARIA labeling for auro-button, deprecate the loadingText property, and update code, documentation, demos, and tests to reflect the new slot API
New Features:
Enhancements:
Documentation:
Tests: