Add close button in htmldialogelement/show/index.md example #42053
Conversation
Example was missing cancel button. Added a close button matching the /en-US/docs/Web/API/HTMLDialogElement/close example
github-actions
bot
added
Content:WebAPI
|
Preview URLs (comment last updated: 2025-11-25 00:27:57) |
leevigraham
changed the title
| <!-- Simple pop-up dialog box, containing a form --> | ||
| <dialog id="favDialog"> | ||
| <form method="dialog"> | ||
| <button type="button" id="close" aria-label="close">X</button> |
There was a problem hiding this comment.
Thank you. Can you change this to be "cancel" and then revert the other changes.
The main reason being that the text before this refers to it as a cancel button, and this is a minimal change to meet the original intent.
There was a problem hiding this comment.
Done. I explicitly set the button label to "Cancel" rather than "X" to show clearer intent.
There was a problem hiding this comment.
Was the original intent to have the Cancel button be the reset button?
<button type="reset">Reset</button>There was a problem hiding this comment.
@leevigraham Thanks! To make this easier to use and test the result, I made this a live example. As part of that I reverted the "X" button and updated the text so that it is clear that "Cancel" and X are the same thing - it just looks better. The extra heading levels are how API examples are supposed to be written according to the current web templates. I also modified the layout a little to kill the arbitrary line breaks.
Was the original intent to have the Cancel button be the reset button?
No, the reset button clears the selection in the dialog. We could talk about this but not sure it is relevant to the specific example. Do you think it would be better to delete that button?
Otherwise I am happy to merge this - just wanted to give you a chance to comment.
There was a problem hiding this comment.
Given this was my first PR I didn't want to overstep with the live example…
Now I've seen a live example and reviewed the other Instance Method docs I've found some more inconsistencies:
- https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/close - Looks good
- https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/requestClose - Could add notes that the
dialog.requestClosemethod triggers thecancelevent - https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/show - needs live example (this PR)
- https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/showModal - Has cancel button… but it's the reset button.
I'm happy to attempt to create a PR that update all the Instance Method docs so the live examples all follow the same layout with javascript implementations reflecting the instance method
There was a problem hiding this comment.
That would be welcome. I'll merge this one though.
Note that you might want to link back to this PR to demonstrate the consistency you are trying to achieve. Reason being that I might not be the reviewer.
There was a problem hiding this comment.
I'll give it a shot :)
github-actions
bot
added
size/xs
github-actions
bot
added
size/s
hamishwillee
left a comment
hamishwillee
left a comment
There was a problem hiding this comment.
Thanks @leevigraham !
|
@hamishwillee I've created a new PR for consolidating the dialog examples if you're interested: #42127 |
2 participants
Description
Example was missing cancel button. Added a close button matching the /en-US/docs/Web/API/HTMLDialogElement/close example
Motivation
Fixing example.
Additional details
N/A
Related issues and pull requests
N/A