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: use semantic elements #341

Merged
merged 5 commits into from
Apr 5, 2023
Merged

Conversation

mfranzke
Copy link
Contributor

@mfranzke mfranzke commented Mar 27, 2023

especially for accessibility reasons; to e.g. have those announced correctly by screenreaders and even also make them reachable on keyboard usage.

Resolves #335

Version

Published prerelease version: v0.8.2-next.0

Changelog

🐛 Bug Fix

Authors: 3

especially for accessibility reasons
@vercel
Copy link

vercel bot commented Mar 27, 2023

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

Name Status Preview Comments Updated (UTC)
chtest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2023 11:37am
playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2023 11:37am

@vercel vercel bot temporarily deployed to Preview – chtest March 27, 2023 10:17 Inactive
@vercel vercel bot temporarily deployed to Preview – chtest March 27, 2023 10:24 Inactive
@vercel vercel bot temporarily deployed to Preview – playground March 27, 2023 10:30 Inactive
@mfranzke mfranzke marked this pull request as ready for review March 28, 2023 09:43
@vercel vercel bot temporarily deployed to Preview – playground March 28, 2023 09:47 Inactive
@vercel vercel bot temporarily deployed to Preview – chtest March 28, 2023 09:49 Inactive
@pomber
Copy link
Contributor

pomber commented Mar 28, 2023

thanks, I'll take a look later.
(maybe you can search for examples of others resetting the button's browser default styles to see if anything is missing)

@vercel vercel bot temporarily deployed to Preview – chtest March 28, 2023 16:38 Inactive
@vercel vercel bot temporarily deployed to Preview – playground March 28, 2023 16:42 Inactive
@mfranzke mfranzke changed the title fix: use semantic element fix: use semantic elements Mar 28, 2023
@Mantish
Copy link
Contributor

Mantish commented Apr 4, 2023

Awesome, this is exactly what we need to fix #335

@pomber
Copy link
Contributor

pomber commented Apr 4, 2023

Sorry for the delay. I'll try to merge it this week.

Comment on lines 107 to 119

&[type="button"] {
background-color: transparent;
border: 1px solid transparent;
color: inherit;
}

svg {
width: 1.5em;
height: 1.5em;
min-width: 1.5em;
min-height: 1.5em;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see that the copy button is also used with other classes like .ch-code-button.
While you can copy these reset styles into the appropriate scss file, another option would be to add the styles directly yo the button on the copy-buton.tsx file. Something like this:

<button
  style={{
    background: "none",
    border: 0,
    color: "inherit",
    fontSize: "inherit",
    padding: 0,
    ...style,
  }}
  ...

@pomber pomber added patch release PR created automatically for new releases labels Apr 5, 2023
@vercel vercel bot temporarily deployed to Preview – chtest April 5, 2023 11:33 Inactive
@vercel vercel bot temporarily deployed to Preview – playground April 5, 2023 11:37 Inactive
@pomber
Copy link
Contributor

pomber commented Apr 5, 2023

I added the button element to a few more buttons and created a reset-button mixin.
Also rolled back the changes to tabs, that's harder and we have another issue for that.

Thank you all.

@pomber pomber merged commit 741d356 into code-hike:next Apr 5, 2023
@github-actions github-actions bot mentioned this pull request Apr 11, 2023
@github-actions
Copy link
Contributor

🚀 PR was released in v0.8.2 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release PR created automatically for new releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

copy button is not accessible
5 participants