[docs] Add radio error demo #19599
[docs] Add radio error demo #19599
Conversation
Details of bundle changes.Comparing: 41fe2e2...997cf00
|
oliviertassinari
changed the title
There was a problem hiding this comment.
Thanks for your interest in these demos. I think that we should go the other way around. The selection controls demos are quite overloaded. The best proof of this is that there are few code previews (demo too long). I think that we should break most of these demos into smaller and unique concerns. Basically, as we do with the text field, one demo = one simple concept.
oliviertassinari
added
the
PR: needs revision
This comment has been minimized.
This comment has been minimized.
|
I added a new demo for showing errors and removed the error demos I added to the other demos. |
|
If this pr is good can you recommend some other demos to update? |
oliviertassinari
requested review from
mbrookes
and removed request for
oliviertassinari
oliviertassinari
added
docs
|
For the Radio Group demo, I wonder if we need the "labelPlacement start" version? There is already a separate Label Placement demo. For the Label Placement demo, you could perhaps make the first radio selected by default. This will give us a regression test for I would also suggest reordering them to put "End" first, and removing the labelPlacement prop, since it is the default.
|
|
I'll make those changes tomorrow. Showing errors was hard for me to understand at first, so I believe it should stay. A common use case for this would be to highlight an incorrect answer on a quiz. If you want me to build that out I can add a button, and when that button is pressed it checks if the right answer is pressed, if not it will show the error. |
|
Interesting. Thanks for the real-world use case. (It's also one of those rare instances where not having a default makes sense.) We need to balance the simplicity of the demos with practical examples. In this case I think it's worth the trade-off of showing a more completely worked example. @oliviertassinari what do you think? |
|
I think that a small validation demo wouldn't harm. Bootstrap includes it: https://getbootstrap.com/docs/4.4/components/forms/#supported-elements. |
The imports were in different orders
|
@mbrookes is something wrong with Argos? |
|
Yep |
|
@mbrookes Is there a way to trigger a re-run of the test so this PR can get approved? |
|
@theswerd I believe we're still waiting for #19599 (comment), so when you push that, it'll be run again. (If it still fails we can merge anyway.) |
|
@mbrookes it looks like its still not working |
oliviertassinari
changed the title
oliviertassinari
force-pushed
the
addErrorDemoToRadioButtons
branch
from
3ace957
to
84356c9
Compare
oliviertassinari
force-pushed
the
addErrorDemoToRadioButtons
branch
from
84356c9
to
0e8c4e0
Compare
There was a problem hiding this comment.
I have updated the pull request to:
- Replace the helper text
Check your answer...with' '. I was confused by the label, I thought something was wrong with my choice, I have later realized that it wanted me to click on the button. But I have no strong feeling that it's better. - Use an actual
<form>element. - Leave the controlled demo unchanged.
- Match the component name match the name of the file.
- Lower cased the header.
Following the lead of the checkbox demos, now the radio button demos show how to show an error.