-
Notifications
You must be signed in to change notification settings - Fork 11
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
Accessibility improvements on club listing page and leaders corner page #293
Conversation
…ove underline from heading-level links
className="club-teaser__image" | ||
/> | ||
</div> | ||
<div className="club-teaser__text"> | ||
<h2 className="club-teaser__title">{title}</h2> | ||
<h2 className="club-teaser__title" id={title.replace(/\s/g, "")}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<h2 className="club-teaser__title" id={title.replace(/\s/g, "")}> | |
<h2 className="club-teaser__title" id={(title||"").replace(/\s/g, "")}> |
Could you safeguard the replace call here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see commit c3be0c3
@@ -13,9 +13,16 @@ export default props => { | |||
<div className={classes}> | |||
<ShadowBox> | |||
<div className="faq__group-header"> | |||
<h2 title="title title--x-small">{props.header}</h2> | |||
<h2 title="title title--x-small" id={props.header.replace(/\s/g, "")}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you safeguard the replace call here from null header?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see commit c3be0c3
<ul className="faq__group-content">{props.children}</ul> | ||
<ul | ||
className="faq__group-content" | ||
aria-labelledby={props.header.replace(/\s/g, "")} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you safeguard the replace call here from null header?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Merge branch 'master' of github.com:ossn/ossn-frontend into accessibility
@@ -15,7 +15,7 @@ | |||
a, | |||
%link { | |||
color: color(link); | |||
text-decoration: none; | |||
// text-decoration: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you delete this instead of commenting it out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, see 690d11c
This is related to issue #263 since it's an accessibility fix.
Commit a409f9a:
Commit a9c7d5b:
Commit b6bda7d: