Skip to content
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

Remove the NonDocumentTypeChildNode mixin #2394

Merged
merged 11 commits into from
Feb 16, 2021

Conversation

Elchi3
Copy link
Member

@Elchi3 Elchi3 commented Feb 16, 2021

NonDocumentTypeChildNode is just a specification construct, aka mixin, and web developers don't interact with it at all.

When I first read the name I wasn't even sure what it does... It is relatively simple though: It adds two properties to two interfaces:

Element.previousElementSibling
Element.nextElementSibling
CharacterData.previousElementSibling
CharacterData.nextElementSibling

Right now these are buried here, though: https://developer.mozilla.org/en-US/docs/Web/API/NonDocumentTypeChildNode
This PR moves the two existing NonDocumentTypeChildNode sub pages under Element and avoids the mixin complexity altogether.

For CharacterData, there are no sub pages for the whole interface right now, so I wasn't feeling motivated to get that work done now. We should document this interface properly one day.

The BCD change happened in mdn/browser-compat-data#9065

@Elchi3 Elchi3 requested review from a team as code owners February 16, 2021 13:06
@Elchi3 Elchi3 requested review from chrisdavidmills and removed request for a team February 16, 2021 13:06
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

Nice work @Elchi3.

I did some light copy edits, and had one query, which I'm not sure what you'll think about — I don't think it needs to block the merging of this PR:

On the API/CharacterData page, there are several uses in the method descriptions of "...when this method returns, data contains..." — is this referring to e.data, i.e. the data available on the event object? If so, which events are relevant in each case? I guess this would be explained more clearly in the pages themselves, when written, but it seems to me that this description itself could be a bit clearer.

@Elchi3
Copy link
Member Author

Elchi3 commented Feb 16, 2021

Thanks Chris, I appreciate you taking a look here so fast!

On CharacterData, I mostly left it as is, see what we have live now: https://developer.mozilla.org/en-US/docs/Web/API/CharacterData I just removed the mention of the ridiculous NonDOcumentTypeChildNode mixin really.

Should I file an issue that proposes to improve/document CharacterData?

@chrisdavidmills
Copy link
Contributor

Thanks Chris, I appreciate you taking a look here so fast!

On CharacterData, I mostly left it as is, see what we have live now: https://developer.mozilla.org/en-US/docs/Web/API/CharacterData I just removed the mention of the ridiculous NonDOcumentTypeChildNode mixin really.

No worries, let's get this merged then.

Should I file an issue that proposes to improve/document CharacterData?

If you could, yes please.

@chrisdavidmills
Copy link
Contributor

Ah, you've got a merge conflict to fix too...

@Elchi3 Elchi3 mentioned this pull request Feb 16, 2021
@Elchi3
Copy link
Member Author

Elchi3 commented Feb 16, 2021

Should I file an issue that proposes to improve/document CharacterData?

If you could, yes please.

--> #2396

@Elchi3
Copy link
Member Author

Elchi3 commented Feb 16, 2021

Conflict solved

@chrisdavidmills chrisdavidmills merged commit 44094b4 into mdn:main Feb 16, 2021
@Elchi3 Elchi3 deleted the demix-NonDocumentTypeChildNode branch February 16, 2021 19:05
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants