Skip to content
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions files/en-us/web/api/htmldialogelement/show/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ button.
<!-- Simple pop-up dialog box, containing a form -->
<dialog id="favDialog">
<form method="dialog">
<button type="button" id="close" aria-label="close">X</button>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@leevigraham leevigraham Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I explicitly set the button label to "Cancel" rather than "X" to show clearer intent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the original intent to have the Cancel button be the reset button?

<button type="reset">Reset</button>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give it a shot :)

<section>
<p>
<label for="favAnimal">Favorite animal:</label>
Expand Down Expand Up @@ -70,7 +71,7 @@ button.

```js
const updateButton = document.getElementById("updateDetails");
const cancelButton = document.getElementById("cancel");
const closeButton = document.getElementById("close");
const dialog = document.getElementById("favDialog");
dialog.returnValue = "favAnimal";

Expand All @@ -89,7 +90,7 @@ updateButton.addEventListener("click", () => {
});

// Form cancel button closes the dialog box
cancelButton.addEventListener("click", () => {
closeButton.addEventListener("click", () => {
dialog.close("animalNotChosen");
openCheck(dialog);
});
Expand Down