Uh oh!
There was an error while loading. Please reload this page.
#79 / Accessibility & Style fixes for Modal - #253
Conversation
| const theme = useTheme(); | ||
| return ( | ||
| <button css={optionStyle({ theme, isEnabled })} disabled={!isEnabled} onClick={onClick}> | ||
| <button aria-label={`Select ${title} Visualizer`} css={optionStyle({ theme, isEnabled })} onClick={onClick}> |
There was a problem hiding this comment.
One callout here
We've added onClick behaviour to display an error message if the button is disabled
A disabled button cannot be clicked, so I had to remove the disabled attribute from the buttons to enable the error message behavior
So I think an aria-disabled should be added here to replace it so we have semantic/accessibility information included
Uh oh!
There was an error while loading. Please reload this page.
| <div slot="content"> | ||
| <p>{infoLabelPercentCopy[key]}</p> | ||
| </div> |
There was a problem hiding this comment.
why do we have a "slot" attribute here? I think this should be encapsulated by the components library that wraps the web components library
There was a problem hiding this comment.
slot is a web component convention and removing the div / slot attribute here also removes the copy.
Encapsulating in the JSX component is a good call.
joneubank
left a comment
There was a problem hiding this comment.
This looks good to me, one change to help simplify.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Accessibility & UI Updates
Depends on changes in: overture-stack/iobio-components#80
Issues
Description of Changes
Readiness Checklist
.env.schemafile and documented in the README