-
Notifications
You must be signed in to change notification settings - Fork 23k
Revise: Web APIs / HTMLDialogElement Docs #42127
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
base: main
Are you sure you want to change the base?
Conversation
* Rewrote html / javascript examples for consistency * Added examples of `returnValue` in close and requestClose * Added additional context around close requests, cancel events
|
@hamishwillee , since you looked at the other one, you have better context for this one than me. But LMK if you don't have time and I will have a look. |
|
Oh, but since you had some questions:
I'm not sure if there are strict rules here. I would link at least the first mention of the thing in the page, and probably the first mention of the thing in that section (i.e. in that example) but not any subsequent mentions. Whether I would include the interface in the link text or not would depend on how obvious it is from the context.
Yes! https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Code_style_guide. And it does actually cover this case:
...so I would say you are OK to use ID attributes here.
This came up quite recently. IMO we should have the detailed documentation on the HTML element page, because the DOM API property is just defined as a reflection of the HTML element. If we document it to the same level in both places we will end up with a LOT of duplication (there are hundreds of these pages). |
I tired to stick to something like that. If I'm referencing the same method on it's page it was
Perfect. I see that the HTML guide says use kebab-case so I'll update the examples.
Was there a GitHub discussion / ticket I can take a look at? I can see it from both sides. DOM API properties are set as attributes. I think it's the case that all attributes have a matching DOM property, but not all DOM properties have a matching attribute? There's always a page for the Element which lists the attributes in a section. This list could be quite long depending on the Element. There's always a page for the DOM Property which is self contained. My expectation is:
Therefore it makes more sense to me that the Element page is the overview of all the attributes with an overview of each attribute, and a link to the DOM Property to dig deeper. This would make the Element page more scannable / readable and allow for more examples on the DOM Property page.
That I agree with :) |
|
Updated all examples to use consistent I've also realised to keep everything in this section consistent I'll also need to revise: https://github.com/mdn/content/blob/main/files/en-us/web/api/htmldialogelement/index.md |
It came up here: https://github.com/mdn/content/pull/40260/files/2d031a2511f2a2a2f2fa9cf770dc7df049925836#r2196957148 . |
|
@hamishwillee @wbamberg This PR is ready for review. I've rewritten all the examples for consistency. This mainly means:
I've expanded the cancel event and return value docs with references to a the spec "close request" |
|
@leevigraham , you don't need to merge main into this PR whenever main changes. |
|
Thanks. I'll look at this today.
FWIW I don't completely agree with this, mostly because I find the HTML docs less easy to digest. Doesn't matter though - this is the agreed thing and trying to change it is not justifiable. I'm not really sure where the level of detail should be in Web API given the above. The doc still needs to be self-contained enough to be useful. I treat the templates as a good guide for broad structure https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Page_structures/Page_types |
| {{APIRef("HTML DOM")}} | ||
|
|
||
| The **`cancel`** event fires on a {{HTMLElement("dialog")}} element when the user instructs the browser that they wish to dismiss the current open dialog. The browser fires this event when the user presses the <kbd>Esc</kbd> key. | ||
| The **`cancel`** event fires on a {{HTMLElement("dialog")}} element when the user sends a [close request](https://html.spec.whatwg.org/multipage/interaction.html#close-request) to the user agent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Linking to the spec is frowned upon, unless absolutely necessary. MDN is supposed to be a human readable version of the spec. Defining the close request below does the job nicely.
- The user doesn't really send anything - the browser does.
- Terminology is mixed, but use browser by preference. I tend to use browser agent for web APIs that also exist in node/deno.
Propose
| The **`cancel`** event fires on a {{HTMLElement("dialog")}} element when the user sends a [close request](https://html.spec.whatwg.org/multipage/interaction.html#close-request) to the user agent. | |
| The **`cancel`** event fires on a {{HTMLElement("dialog")}} element when the user triggers a close request. |
| The **`cancel`** event fires on a {{HTMLElement("dialog")}} element when the user sends a [close request](https://html.spec.whatwg.org/multipage/interaction.html#close-request) to the user agent. | ||
|
|
||
| This event is cancelable but can not bubble. | ||
| Examples of close requests are: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Examples of close requests are: | |
| Close requests might be triggered by: |
| div { | ||
| margin: 0.5rem; | ||
| #status-text { | ||
| height: 120px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider doubling this as well
| The following example shows a button that, when clicked, opens a {{htmlelement("dialog")}} via the {{domxref("HTMLDialogElement.showModal()", "showModal()")}} method. | ||
|
|
||
| From there you can trigger the `cancel` event by either clicking _Request Close_ button to close the dialog (via the {{domxref("HTMLDialogElement.requestClose()", "requestClose()")}} method), or press the <kbd>Esc</kbd> key. | ||
|
|
||
| Preventing the dialog from closing is demonstrated with a checkbox. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is obviously a lot better than no explanation!
If I am doing a method-specific document, I tend to structure like "
This example shows how to cancel a dialog, and how you can also prevent a dialog from closing by ...
Then either have detailed "how" or interleave in the text.
However in this case we have a number of examples that seem to be the same - such as requestClose. In that case I might keep your current structure, but in requestClose() remove the example, and just have "See the cancel event example for blah blah).
Thoughts?
| #### Result | ||
|
|
||
| {{ EmbedLiveSample('Canceling a dialog', '100%', '100px') }} | ||
| {{ EmbedLiveSample('Canceling a dialog', '100%', '250px') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, I'm showing Chrome sending a close event for the Escape key, while FF follows the spec and sends a cancel events. We should update the compatibility data to reflect that.
| The following example shows a button that, when clicked, opens a {{htmlelement("dialog")}} via the {{domxref("HTMLDialogElement.showModal()", "showModal()")}} method. | ||
| From there you can click the either _Close_ button to close the dialog (via the `close()` method). | ||
|
|
||
| The _Close_ button closes the dialog without a {{domxref("HTMLDialogElement.returnValue", "returnValue")}}, while the _Close w/ return value_ button closes the dialog with a {{domxref("HTMLDialogElement.returnValue", "returnValue")}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template likes a level 3 heading for Web API examples, or it should :-)
| The following example shows a button that, when clicked, opens a {{htmlelement("dialog")}} via the {{domxref("HTMLDialogElement.showModal()", "showModal()")}} method. | |
| From there you can click the either _Close_ button to close the dialog (via the `close()` method). | |
| The _Close_ button closes the dialog without a {{domxref("HTMLDialogElement.returnValue", "returnValue")}}, while the _Close w/ return value_ button closes the dialog with a {{domxref("HTMLDialogElement.returnValue", "returnValue")}}. | |
| ### Closing a dialog | |
| The following example demonstrates how you can close a dialog, optionally passing a return value. | |
| It displays a button that can be clicked to open a {{htmlelement("dialog")}} by calling the {{domxref("HTMLDialogElement.showModal()", "showModal()")}} method. | |
| The dialog that is displayed can be closed by clicking either _Close_ button. | |
| The _Close_ button closes the dialog without a {{domxref("HTMLDialogElement.returnValue", "returnValue")}}, while the _Close w/ return value_ button closes the dialog with a {{domxref("HTMLDialogElement.returnValue", "returnValue")}}. |
| The **`close()`** method of the {{domxref("HTMLDialogElement")}} interface closes the {{htmlelement("dialog")}}. | ||
| An optional string may be passed as an argument, updating the `returnValue` of the dialog. | ||
| An optional string may be passed as an argument, updating the {{domxref("HTMLDialogElement.returnValue", "returnValue")}} of the dialog. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth noting what event(s) is/are fired? Can closing be prevented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and adding those relevant events as See also section at end.
|
|
||
| ### Live example | ||
|
|
||
| #### HTML |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change ### Live example above to a better name - up to you - but perhaps "Handling close events". Note that the example macro needs a corresponding name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you want to explain it? In particular perhaps link to information on <form method="dialog"> since it isn't clear why closing via method dialog is interesting, other that we have a button for it.
|
|
||
| - `any` | ||
| - : The dialog can be dismissed with a light dismiss user action, a platform-specific user action, or a developer-specified mechanism. | ||
| - : The dialog can be dismissed by a [close request](https://html.spec.whatwg.org/multipage/interaction.html#close-request) or clicks outside the dialog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No spec links
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I would not purge the term "light dismiss" - its in the spec and other docs. You could define it if you want.
I'd be tempted to revert this. If you want the pattern then maybe define earlier what the different close options are, such as close request and light dismiss and platform-specific user action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto below
| - : The dialog can only be dismissed by a [close request](https://html.spec.whatwg.org/multipage/interaction.html#close-request). | ||
| - `none` | ||
| - : The dialog can only be dismissed with a developer-specified mechanism. | ||
| - : No user actions automatically close the dialog. The dialog can only be dismissed with a developer-specified mechanism. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An improvement, but perhaps be even more specific - such as
| - : No user actions automatically close the dialog. The dialog can only be dismissed with a developer-specified mechanism. | |
| - : The dialog can only be dismissed with a developer-specified mechanism. User actions such as clicking outside the dialog have no effect. |
|
|
||
| - {{domxref("HTMLDialogElement.closedBy")}} | ||
| - : A string that sets or returns the [`closedby`](/en-US/docs/Web/HTML/Reference/Elements/dialog#closedby) attribute value of the `<dialog>` element, which indicates the types of user actions that can be used to close the dialog. | ||
| - : A string that sets or returns the [`closedby`](/en-US/docs/Web/HTML/Reference/Elements/dialog#closedby) HTML attribute, indicating the types of user actions that can be used to close the dialog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps remove "user", since some are not user actions.
| - : A string that sets or returns the [`closedby`](/en-US/docs/Web/HTML/Reference/Elements/dialog#closedby) HTML attribute, indicating the types of user actions that can be used to close the dialog. | |
| - : A string that sets or returns the [`closedby`](/en-US/docs/Web/HTML/Reference/Elements/dialog#closedby) HTML attribute, indicating the types of actions that can be used to close the dialog. |
|
|
||
| - {{domxref("HTMLDialogElement.close()")}} | ||
| - : Closes the dialog. An optional string may be passed as an argument, updating the `returnValue` of the dialog. | ||
| - : Closes the dialog. An optional string may be passed as an argument, updating the {{domxref("HTMLDialogElement.returnValue", "returnValue")}} of the dialog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might instead link to the anchor #htmldialogelement.returnvalue of the item above. There is no standard on this, but I have a slight preference to stay on the same page if I can get the information just above. Entirely up to you.
| - : Displays the dialog modelessly, i.e., still allowing interaction with content outside of the dialog. | ||
| - {{domxref("HTMLDialogElement.showModal()")}} | ||
| - : Displays the dialog as a modal, over the top of any other dialogs that might be present. Everything outside the dialog are [inert](/en-US/docs/Web/API/HTMLElement/inert) with interactions outside the dialog being blocked. | ||
| - : Displays the dialog as a modal, over the top of any other dialogs that might be present. Everything outside the dialog is {{DOMxRef("HTMLElement.inert", "inert")}} with interactions outside the dialog being blocked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, Web API does not care if you use the link form or the macro. So you could have just done inert as the change.
|
|
||
| - {{domxref("HTMLDialogElement/cancel_event", "cancel")}} | ||
| - : Fired when the dialog is requested to close, whether with the escape key, or via the `HTMLDialogElement.requestClose()` method. | ||
| - : Fired when the dialog is requested to close, whether with the escape key, or via the {{domxref("HTMLDialogElement.close()", "requestClose()")}} method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean?
| - : Fired when the dialog is requested to close, whether with the escape key, or via the {{domxref("HTMLDialogElement.close()", "requestClose()")}} method. | |
| - : Fired when the dialog is requested to close, whether with the escape key, or via the {{domxref("HTMLDialogElement.requestClose()", "requestClose()")}} method. |
| - : Fired when the dialog is requested to close, whether with the escape key, or via the {{domxref("HTMLDialogElement.close()", "requestClose()")}} method. | ||
| - {{domxref("HTMLDialogElement/close_event", "close")}} | ||
| - : Fired when the dialog is closed, whether with the escape key, the `HTMLDialogElement.close()` method, or via submitting a form within the dialog with [`method="dialog"`](/en-US/docs/Web/HTML/Reference/Elements/form#method). | ||
| - : Fired when the dialog is closed, whether with the escape key, the {{domxref("HTMLDialogElement.close()", "close()")}} method, or via submitting a form within the dialog with [`method="dialog"`](/en-US/docs/Web/HTML/Reference/Elements/form#method). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it fired for requestClose()? I "think" that close fires the close event, whereas the requestClose fires the cancellable cancel event, which then results in the close event if not prevented. That should be captured in all this.
| ```css hidden | ||
| #log { | ||
| height: 150px; | ||
| #status-text { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest you revert the change to status-text everywhere. The live example guide suggests #log and it is all over the docs. Changing it therefore makes things less readable for people who are working in the source - and it isn't visible to users anyway.
|
|
||
| The following example shows a simple button that, when clicked, opens a {{htmlelement("dialog")}} containing a form via the `show()` method. | ||
| From there you can click the _Cancel_ button ("X") to close the dialog (via the {{domxref("HTMLDialogElement.close()")}} method), or submit the form via the submit button. | ||
| The following example shows a simple button that, when clicked, opens a {{htmlelement("dialog")}} via the `show()` method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the show and showmodal examples I think it is an improvement to show an actual "basic usage" without the selection.
What I think is problematic, and it predates your changes, is that neither example highlights the difference between modal and non-modal dialogs.
hamishwillee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much @leevigraham - lots of improvements. I've added some ways of working comments that aren't captured in the guides.
|
Thanks for the detailed feedback @hamishwillee. It will better help me understand writing docs in a consistent MDN style. I'll go through it all in the next couple of days. |
Co-authored-by: Hamish Willee <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Description
returnValuein close and requestCloseThe examples are now much simpler focussing on the api methods. I've removed a lot of form related cruft which may have been confusing to developers new to the APIs.
I'm pretty sure the examples were originally copied from https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/dialog#examples
If this PR is successful I'll start on consolidating the examples in the https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/dialog docs.
Motivation
After finding a broken example in the docs (#42053) I found more inconsistencies in the examples across all the Web APIs / HTMLDialogElement docs.
Additional details
Questions for reviewers… this is my first major PR.
Is there a style guide for when to use links vs code blocks for api methods in related docs? I've seen examples of both
returnValue,{{domxref("HTMLDialogElement.returnValue", "returnValue")}}, and {{domxref("HTMLDialogElement.returnValue")}}.Is there a style guide for writing code examples? In this case I've stuck with id selectors from the first example I updated, this also makes selection a bit clearer for new developers IMO, but it also may set a bad precedent.
Should the details page for a property include more information that the property on the element page. Example: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/dialog#closedby contained more information about how the property worked than the instance property: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/closedBy. Should these docs be rewritten to reflect each other?
Related issues and pull requests
#42053
// ping @hamishwillee