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

Semantic HTML improvements #343

Open
wants to merge 8 commits into
base: feature/react-18-useid
Choose a base branch
from

Conversation

sarah-j-lancaster
Copy link

@sarah-j-lancaster sarah-j-lancaster commented Dec 16, 2021

Keen to merge into the React 18 branch so the changes can go out in a major together?
Open to maybe also changing the prop name because now it seems a bit weird?

  • Replace divs with roles heading and button with the semantic HMTL.
  • Updated unit and integration tests
  • Updated the demo styles to remove those added by the native elements

demo/src/main.css Outdated Show resolved Hide resolved
cursor: pointer;
display: inline-block;
padding: 10px 0;
color: var(--colorPageLinks);
background-color: transparent;
font: 16px/1.5 sans-serif;
font-weight: bold;

Choose a reason for hiding this comment

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

AI guess there's a question of how styled this should be. Are our current buttons bold? https://react-accessible-accordion.springload.co.nz/

Copy link
Author

Choose a reason for hiding this comment

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

This is the Show code button css, yep its bold, I just moved the rule down to be with the other text ones

DivAttributes,
Exclude<keyof DivAttributes, keyof InjectedButtonAttributes>
ButtonAttributes,
Exclude<keyof ButtonAttributes, keyof InjectedButtonAttributes>

Choose a reason for hiding this comment

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

Good to rename all these 👍

src/css/fancy-example.css Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Dec 19, 2021

Coverage Status

Coverage decreased (-0.1%) to 87.786% when pulling 9e12c09 on semantic-buttons into 47b5d89 on feature/react-18-useid.

@sarah-j-lancaster sarah-j-lancaster force-pushed the semantic-buttons branch 2 times, most recently from dcb28e3 to 9e23bb6 Compare December 19, 2021 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants