-
Notifications
You must be signed in to change notification settings - Fork 367
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
Use button
instead of a
for closing modals
#446
Comments
button
instead of a
for closing modals
@Atmos4, But we must avoid breaking changes because we are no longer in beta or release-candidate.
Sure, it would be great if you could open a PR. Note, you also need to reset (EDITED) |
@lucaslarroche Out of curiosity, how much does bundle size matter to this project? If it is important, then I might just go with the first solution I described. It should be doable with I edited the codepen to reflect what I described above, and simplify the CSS to work with |
Semantically valid is more important than the bundle size.
Agreed (EDITED) |
PR is ready, see #448 |
@Atmos4, I edited my comments about targeting So you were right, |
@lucaslarroche it is! Check out the MDN article and this codepen. The original codepen I posted above was also containing this solution. |
Great. I missed it when researching yesterday.
Yes. That's why I suggested at some point to target |
@lucaslarroche The problem with using <dialog>
<article>
<!-- Modal content -->
<footer>
<form>
<button formmethod=dialog>Cancel</button> <!-- this button should look normal! -->
</form>
</footer>
</article>
</dialog> By the way, is there a reason to have some part of the icon specific code coupled to the header? It could be interesting to allow usage of that icon anywhere inside the article, for example like this: <dialog id=modal1>
<article>
<button rel=prev onclick="modal1.close()"></button> <!-- currently doesn't float:right -->
<!-- Modal content -->
</article>
</dialog> I can create an issue/PR to decouple the code if you are interested. Maybe there are valid reasons to have it otherwise though! |
Describe the issue
The usage of
button
for closing modals makes a lot more sense thana
.Enter
key presses. Buttons respond to bothEnter
andSpace
.href
attribute in order to be accessible. This forces the usage ofhref=#
, which can confuse screen readers and will in fact perform the scroll animation unless theonclick
handler returnsfalse
, which further increases verbosity.Solutions
One solution would be to recommend
button
as a way to close modals, and properly style any button withclass=close
(orrel=prev
even though it is not valid on buttons, and also a non-standard way to describe dialog closing actions).Extending the previous solution for classless pico, we can use
<form method=dialog>
HTML element, which is a non-Javascript solution to closing dialogs. Alternatively if the button is already in a form, we can use<button formmethod=dialog>
.Here is a demo of the extended solution on CodePen.
If you agree on the issue, let me know which solution you like best and I can start working on a PR!
The text was updated successfully, but these errors were encountered: