Use aria-describedby on checkboxes with descriptions #2079
Conversation
✅ Deploy Preview for jsonforms-examples ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Thanks for the contribution! |
|
@sdirix No problem! I need to make the same type of change for text inputs, but I wanted to make sure this approach (passing along the inputProps) is OK with you. Should I proceed? |
There was a problem hiding this comment.
Thanks again! The code looks good.
I thought about whether it's worth to useMemo the inputProps objects but I actually think it's not necessary here. In almost all use cases the memo will not apply.
The only use case I could think of is when someone updates the JSON Schema at runtime with a new description. This is really an edge case, and if someone does that they probably don't care that the input is also rerendered (maybe there is even some additional memo barrier further within Material UI)
So the code looks good! However it would be nice if you could add test cases
|
@brockfanning Will you add a testcase? |
|
@sdirix I just pushed up a couple of tests - please let me know if that is not what you had in mind. I only did tests for MaterialBooleanToggleControl because I did not see any existing tests for MaterialBooleanControl. Did I miss those? |
|
Thanks for the contribution ❤️ ;) |
Fixes #2078
I should note that the problem this solves also affects other components, like the text input. But I wanted to start with a simpler case (checkboxes) before tackling that one, to make sure the team is OK with this approach.