-
Notifications
You must be signed in to change notification settings - Fork 0
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
AIS-64 | @jdwjdwjdw | A11y and console fixups #153
Conversation
✅ Deploy Preview for decanter ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -9,7 +9,7 @@ lastUpdatedDate: 2022-04-06T12:00:00.000Z | |||
<div class="h-full w-full overflow-hidden absolute"> | |||
<img | |||
class="h-full w-full object-cover object-center" | |||
src="https://placekitten.com/1000/750" |
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.
Ah, boo. No more place kitteh.
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.
I was sad too 😿
content/examples/cards.mdx
Outdated
@@ -200,29 +200,29 @@ lastUpdatedDate: 2023-05-25T12:00:00.000Z | |||
</Section> | |||
<Section heading="Library Card" width="full"> | |||
```html | |||
<div class="card block w-full basefont-23 leading-display bg-white text-black border border-solid border-black-10 shadow-md md:max-w-500"> | |||
<div class="overflow-hidden aspect-[16/9] relative" aria-hidden="true"> | |||
<div className="card block w-full basefont-23 leading-display bg-white text-black border border-solid border-black-10 shadow-md md:max-w-500"> |
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.
@jdwjdwjdw
It looks like you've changed the example markup from HTML valid markup to React markup. Can you tell me more about why you changed these? This changes the example markup code and they are invalid HTML attributes.
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.
The stuff inside the html fencing '''html
should stay as html and not react.
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.
Thanks - I got a little overambitious while trying to fix the console errors. Some of the JSX vs HTML did need a little cleanup, so I removed my incorrect changes and kept the ones that needed updating. Should be good for another look now.
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.
Thanks for adding the labels.
For the HTML code inside the code fence, please return it to HTML markup.
Otherwise this all looks good.
NOT READY FOR REVIEW
Summary
Form field missing a label
A11y issues at https://decanter.stanford.edu/examples/form-elementsReview By (Date)
Criticality
Review Tasks
Setup tasks and/or behavior to test
/examples/form-elements
, run the Sitemprove extension, and confirm that theForm field missing a label
A11y issues are no longer being flagged on that page.Associated Issues and/or People