-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
Editcontext API documentation #31176
Conversation
Update: this PR isn't ready for review yet. It only contains the boilerplate code for each interface, method, property, event in the spec. My next step will be to fill in the main API concept, and then slowly fill in the reference pages one by one. |
Update: the intro/concept text is now ready in the API landing page. And I've filled in about half of the reference pages. Next step is to finish filling everything in. |
Update: almost done with all reference pages. About 3 remaining. |
This is pretty cool! I like how it also makes use of some other interesting APIs like custom selection etc. It would be really cool to have a guide page or a tutorial that shows how to do interesting things by combining these APIs.
Examples on MDN are still a bit of a mess right now. I think the closest thing to what you have done is https://github.com/mdn/dom-examples. These examples there can be embedded into MDN pages.
I'm not sure. I think we didn't really like all the glitch demo links some authors provided in the past but we did allow them. So I would say it would be okay to link to the external demo even though it's not ideal. |
There are 3 pages missing. The constructors for the events:
|
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.
Some code doesn't work. Need to decide if it is e
or event
. Looks like you used e
more.
files/en-us/web/api/editcontext/textformatupdate_event/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/textformatupdateevent/gettextformats/index.md
Outdated
Show resolved
Hide resolved
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 for adding the event constructors! Some tweaks needed to the syntax box. Need to also have a line for usage without the options
parameter. See https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Page_structures/Syntax_sections
files/en-us/web/api/characterboundsupdateevent/characterboundsupdateevent/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/textformatupdateevent/textformatupdateevent/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/textformatupdateevent/textformatupdateevent/index.md
Outdated
Show resolved
Hide resolved
One more general comment on the code examples (which might not be actionable, but I wanted to mention it): For some of them I was able to paste the HTML and JS snippets into a codepen and actually saw something happening. Like when I type some text and there is an For other examples (e.g. those with a (I know it can be hard to author short code examples for reference pages, but I think they are almost always better when they can show something, even if it is just logging.) |
That's a very good point and I 100% agree that standalone code samples that just work, and visibly do something, is far better. It might not always be possible though. For example, for Also, even if the EditContext API is useful, it does require a lot of code for it to do anything. So, some of these examples, for the sake of being short, don't really achieve much. But, anyway, I'll go over them all and try my best to make them more visual and as standalone as they can be. |
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
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 Patrick, great work! I made a final pass through all of the reference pages and have some more comments but after this the reference pages should be good to go.
As you say, it is not always possible to trigger the events. I was testing the code you provided by dispatching the events manually. For example:
const testEvent = new CharacterBoundsUpdateEvent("characterboundsupdate", {rangeStart: 0, rangeEnd: 3});
editContext.dispatchEvent(testEvent);
(I think such code could be example code for the event constructor pages, but of course using event constructors and dispatching events manually isn't something you'd normally do)
files/en-us/web/api/characterboundsupdateevent/characterboundsupdateevent/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/characterboundsupdateevent/characterboundsupdateevent/index.md
Show resolved
Hide resolved
files/en-us/web/api/editcontext/characterboundsrangestart/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/textformatupdateevent/textformatupdateevent/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/textformatupdateevent/textformatupdateevent/index.md
Show resolved
Hide resolved
One more thing: BCD marks this API as experimental (because it is in just one engine). If you don't mark things as experimental in this PR, I think Onkar's bot will update all this PR's files automatically in a follow-up PR (adding the |
Co-authored-by: Florian Scholz <fs@florianscholz.com>
I added the experimental status and seecompattable macro. I'm not quite sure where the experimental_inline macro should go, so I left that out. It seems like it needs to go in front of each and every method, event, property, and constructor in a reference page. I wasn't sure, so I didn't want to do all this work for nothing, especially if we have a tool to do it. |
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 @captainbrosset! 👍 I'm happy with this! I will merge it, so all of there reference pages can land.
The inline experimental macros can be added by Onkar's automation, you're right to not necessarily worry about them now.
We also haven't talked further about where to put a more extensive example (or a link to it) but I think that could also be a follow-up.
Let's get this in! Great work!
Thank you Florian for the great reviews and help. |
STATUS: This PR has already been technically reviewed by engineers who worked on the API. Editorial review is ongoing.
Description
This PR adds the necessary new pages for documenting the EditContext API.
Motivation
EditContext just shipped in Chromium browsers, and the corresponding data is already in BCD. It's time to document the API on MDN for people to know how to use it.
Additional details
https://w3c.github.io/edit-context/
Related issues and pull requests
mdn/mdn#493