Skip to content

feat: add title and aria-label attributes to btn-icon components #3619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Aug 9, 2025
Merged

feat: add title and aria-label attributes to btn-icon components #3619

merged 5 commits into from
Aug 9, 2025

Conversation

JamesN-dev
Copy link
Contributor

@JamesN-dev JamesN-dev commented Jul 27, 2025 •
edited by endigo9740
Loading

Linked Issue

Closes #3596

Description

Adds title and ariaLabel props to all btn-icon components in codebase.
Provides semantic default values and updates examples with a11y attributes.

Checklist

Please read and apply all contribution requirements.

  • Your branch should be prefixed with: docs/, feature/, chore/, bugfix/
  • Contributions should target the main branch
  • Documentation should be updated to describe all relevant changes
  • Run pnpm check in the root of the monorepo
  • Run pnpm format in the root of the monorepo
  • Run pnpm lint in the root of the monorepo
  • Run pnpm test in the root of the monorepo
  • If you modify /package projects, please supply a Changeset

Changesets

View our documentation to learn more about Changesets. To create a Changeset:

  1. Navigate to the root of the monorepo in your terminal
  2. Run pnpm changeset and follow the prompts
  3. Commit and push the changeset before flagging your PR review for review.

@JamesN-dev
feat: add title and aria-label attributes to btn-icon components
7a0c638 
- Add accessibility props with default values to components identified in issue
- Update examples and playgrounds with accessibility attributes

Fixes skeletonlabs#3596
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jul 27, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: a34c536

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@skeletonlabs/skeleton-svelte Minor
@skeletonlabs/skeleton-react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel Vercel
Copy link

vercel bot commented Jul 27, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
www.skeleton.dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2025 5:14pm

@JamesN-dev
Copy link
Contributor Author

JamesN-dev commented Jul 28, 2025 •
edited
Loading

I initially missed wiring the accessibility props (btnDismissTitle and btnDismissAriaLabel) into the Toast components for both Svelte and React, so I committed those fixes just now.

Does the API reference in the Toast docs need to explicitly include these props? It seems the API tables might be generated during a build script, so I left those files untouched in case they pulled from the JSDocs comments. Can update if needed.

From what I can tell, Toast/Toaster are the only fully “public” components in this set. The rest being examples and website use. I did expose props on components used in the website like HomeCompGrid, Controls, and Lightswitch. Those are not part of the public API but, still being components, I updated them the same way for consistency.

I was torn whether to create a separate types file for the website’s Lightswitch component (sites/themes.skeleton.dev/src/lib/components/common/Lightswitch/Lightswitch.svelte). These types could also be inlined in the component. I can submit a fix if you’d like.

Also, not sure where you are on testing for a11y, but if you prefer, I can add prop tests (btnDismissTitle, btnDismissAriaLabel) to the component test. Let me know and I’ll include them.

@endigo9740
Copy link
Contributor

@JamesN-dev per above

Does the API reference in the Toast docs need to explicitly include these props? It seems the API tables might be generated during a build script, so I left those files untouched in case they pulled from the JSDocs comments. Can update if needed.

Correct, API Reference tables are auto-generated based on the types.ts file co-located with each component. As long as this has been updated accordingly, the table will update too. See the preview link generated by Vercel above to preview this.

From what I can tell, Toast/Toaster are the only fully “public” components in this set. The rest being examples and website use. I did expose props on components used in the website like HomeCompGrid, Controls, and Lightswitch. Those are not part of the public API but, still being components, I updated them the same way for consistency.

That sounds great, both were part of the reqs in the original ticket.

I was torn whether to create a separate types file for the website’s Lightswitch component (sites/themes.skeleton.dev/src/lib/components/common/Lightswitch/Lightswitch.svelte). These types could also be inlined in the component. I can submit a fix if you’d like.

I'll provide some direct feedback in this in my code review shortly.

Also, not sure where you are on testing for a11y, but if you prefer, I can add prop tests (btnDismissTitle, btnDismissAriaLabel) to the component test. Let me know and I’ll include them.

I don't think this is needed. This is a temporary solution until the new component APIs are introduced in Skeleton v4. At which point users will have full control over these values in a more direct fashion.

endigo9740
endigo9740 requested changes Jul 29, 2025
View reviewed changes
Copy link
Contributor

@endigo9740 endigo9740 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JamesN-dev overall great job, just a few comments. This likely won't take long, it's just the same few changes repeated in a few places. I've tried to add context for the requests as I go, but feel free to ask questions if something is unclear.

@endigo9740
Copy link
Contributor

@JamesN-dev I'd love to have an ETA for when the requested changes might be completed. If you don't feel like you can complete these, please let us know.

@JamesN-dev
Copy link
Contributor Author

@endigo9740 Apologies for the delayed response. I'm trying to find the time to square these up. I should be able to get to it sometime by the end of the week. Will try to make it sooner than later!

@JamesN-dev
Copy link
Contributor Author

@endigo9740 Sorry for the delay. All feedback and requested changes have been addressed.

@JamesN-dev JamesN-dev force-pushed the feature/a11y-icon-props branch from 76708d6 to 6b9e311 Compare August 9, 2025 02:10
@endigo9740 endigo9740 merged commit df1eef4 into skeletonlabs:main Aug 9, 2025
8 checks passed
@github-actions github-actions bot mentioned this pull request Aug 9, 2025
Open
@JamesN-dev JamesN-dev deleted the feature/a11y-icon-props branch August 9, 2025 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@endigo9740 endigo9740 endigo9740 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add title/aria-label attribute to components that include icon buttons
2 participants
@JamesN-dev @endigo9740