Skip to content
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.

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

fix: sd badge a11y #1563

Merged
merged 19 commits into from
Oct 31, 2024
Merged

fix: sd badge a11y #1563

merged 19 commits into from
Oct 31, 2024

Conversation

smfonseca
Copy link
Contributor

Description:

Addresses the following tickets:

Definition of Reviewable:

  • Documentation is created/updated
  • Migration Guide is created/updated
  • E2E tests (features, a11y, bug fixes) are created/updated
  • Stories (features, a11y) are created/updated
  • relevant tickets are linked

Copy link

github-actions bot commented Oct 18, 2024

🚀 Storybook has been deployed for branch fix_sd-badge-a11y

@miguelxnxn
Copy link

miguelxnxn commented Oct 23, 2024

I think it's incoherent to describe the different colour variants as "not having a semantic meaning" while also naming them "default", "success" and "error".

Whether the variant is defined semantically in Storybook/Figma or not, the use of certain colours (green and red) does impact how the user perceives the urgency of a badge, especially if several variants are used within the same context. Therefore, I suggest acknowledging colour interpretations or otherwise neutral colour names or brand-related terms.

Some examples:

  • blue: "default", "blue"
  • red: "alert", "urgent", "red", "high-prio"
  • green: "accent", "green", "low-prio", "subtle", "positive"

@coraliefeil
Copy link
Contributor

As far as I understand it the variant name is the issue and not the token name right?

sd-badge is the only component where this issue occurs ... changing the token name is rather hectic and I’m wondering if we could solve it pragmatically by just naming the color variants to "blue", "red", "green".

@miguelxnxn
Copy link

The token name wouldn't be the issue (from my perspective). Naming the variants by the colour names is a good solution if we don't want to define connotations there. I would warn about colour interpretation in the documentation.

@smfonseca
Copy link
Contributor Author

smfonseca commented Oct 24, 2024

We can change the variants name although this is a breaking change, therefore would need to tackle in a separate branch (FYI @mariohamann). @miguelxnxn can you then provide me a text to include in the variants description?
Ticket for updating the variants names is created.

@miguelxnxn
Copy link

Since there is no particular desire to encourage the use of the blue one and therefore set it as "default", I think "blue", "red" and "green" are the best options to avoid connotations. @smfonseca

karlbaumhauer
karlbaumhauer previously approved these changes Oct 24, 2024
@MartaPintoTeixeira
Copy link
Contributor

Screenshot 2024-10-30 at 17 19 44 In safari the md size number is not vertically aligned in the circle. Is this a browser issue? or can we fix it?

@smfonseca
Copy link
Contributor Author

Screenshot 2024-10-30 at 17 19 44 In safari the md size number is not vertically aligned in the circle. Is this a browser issue? or can we fix it?

Seems that the line-height was messing up the alignment in Safari. Should be fixed now, please check again 🔎@MartaPintoTeixeira.

mariohamann
mariohamann previously approved these changes Oct 31, 2024
@MartaPintoTeixeira
Copy link
Contributor

MartaPintoTeixeira commented Oct 31, 2024

Please change this copy on the 2nd template:
"Examples of sd-navigation-item working with sd-badge:"
change to: "Example of sd-navigation-item working with sd-badge:"

@MartaPintoTeixeira
Copy link
Contributor

@MartaPintoTeixeira
Copy link
Contributor

In figma we have "sd-status-badge" on the related components. Can you add it?

@smfonseca
Copy link
Contributor Author

In figma we have "sd-status-badge" on the related components. Can you add it?

The sd-status-badge doesn't exist in code yet. It's planned for iteration 18

@smfonseca
Copy link
Contributor Author

In figma we link to 4 Related Templates. https://www.figma.com/design/YDktJcseQIIQbsuCpoKS4V/Component-Docs?node-id=2116-4928&node-type=frame&t=GObamI9yCWkPNV4b-0

Please double check.

The status badge with text template doesn't exist yet, but there's a ticket in the backlog to implement it.
The tab with badge is a template which @auroraVasconcelos is still working on, therefore I would ask @auroraVasconcelos to please link the template in the sd-badge once she's done.

@smfonseca
Copy link
Contributor Author

Please change this copy on the 2nd template: "Examples of sd-navigation-item working with sd-badge:" change to: "Example of sd-navigation-item working with sd-badge:"

Updated

mariohamann
mariohamann previously approved these changes Oct 31, 2024
mariohamann
mariohamann previously approved these changes Oct 31, 2024
@smfonseca smfonseca merged commit 762656f into main Oct 31, 2024
12 of 13 checks passed
@smfonseca smfonseca deleted the fix/sd-badge-a11y branch October 31, 2024 17:19
auroraVasconcelos pushed a commit that referenced this pull request Nov 4, 2024
# [@solid-design-system/components-v3.20.5](components/3.20.4...components/3.20.5) (2024-10-31)

### Bug Fixes

* sd badge a11y ([#1563](#1563)) ([762656f](762656f))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

9 participants