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

Add mouse events #4019

Merged
merged 7 commits into from
May 2, 2019
Merged

Add mouse events #4019

merged 7 commits into from
May 2, 2019

Conversation

irenesmith
Copy link
Contributor

This PR relates to MDN/Sprints #955 - the refactoring of mouse events.

Irene Smith and others added 4 commits November 9, 2018 10:11
Updating the code in my fork
Merge latest MDN changes into my repository
Update my fork from original
@irenesmith irenesmith mentioned this pull request Apr 29, 2019
23 tasks
@Elchi3 Elchi3 added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Apr 29, 2019
Copy link
Collaborator

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

Whoops! Looks like there's a little syntax error.

api/Element.json Outdated Show resolved Hide resolved
@queengooborg queengooborg dismissed their stale review May 1, 2019 17:53

Syntax error fixed!

api/Element.json Outdated
}
}
},
"mousewheel": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "mousewheel_event"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mousewheel fixed to mousewheel_event

@a2sheppy
Copy link
Contributor

a2sheppy commented May 1, 2019

Around line 3767 or so, inside the info for getElementsByTagNameNS, is all_elements_selector. Inside that is the description getElementsByTagName(*). I'm pretty sure that should be getElementsByTagNameNS(*).

In addition, that block is missing information for several browsers, resulting in an error when using npm run render api.Element to test the data.

Copy link
Contributor

@a2sheppy a2sheppy left a comment

Choose a reason for hiding this comment

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

Please correct the issues commented upon, then I will resume testing.

api/Element.json Outdated
"description": "<code>mousewheel</code> event",
"support": {
"chrome": {
"version_added": true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wasn't able to track down the exact commit, but Chrome Status indicates this was in 31 on desktop only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thanks!

api/Element.json Outdated
"support": {
"chrome": {
"version_added": "1"
},
"chrome_android": {
"version_added": true
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be "18".

Copy link
Contributor

@a2sheppy a2sheppy left a comment

Choose a reason for hiding this comment

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

LGTM, once Joe's last update to the Chrome Android version number for getElementsByTagNameNS(*) is changed from true to 18.

Per Joe Medley's comment, I've updated the version all_elements_selector to 18 for chrome_android.
Copy link
Contributor

@a2sheppy a2sheppy left a comment

Choose a reason for hiding this comment

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

👍

@a2sheppy a2sheppy merged commit 7dd77fb into mdn:master May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants