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

Update triggerEvent & triggerKeyEvent target type #1103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eramod
Copy link

@eramod eramod commented Aug 16, 2021

Include Document in the target argument type for triggerEvent and triggerKeyEvent, as it's already included in the TS code.

function getElement(target: Document): Document;

Once this Docs PR is merged, we'll merge this DefinitelyTyped PR DefinitelyTyped/DefinitelyTyped#55123 since this repo is not publishing types yet.

@stefanpenner
Copy link
Member

Thank you!

@@ -1205,3 +1205,5 @@ Returns **([Array][68]\<Warning> | [Promise][64]<[Array][68]\<Warning>>)** An ar
[75]: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode

[76]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Statements/function

[77]: https://developer.mozilla.org/en-US/docs/Web/API/Document
Copy link
Member

Choose a reason for hiding this comment

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

Was this change generated by yarn run docs? If not, we likely need to investigate why yarn run docs isn't producing this change, as API.md is an artifact of that build step

Copy link
Author

@eramod eramod Aug 18, 2021

Choose a reason for hiding this comment

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

@stefanpenner I was asked to update the docs before merging this Definitely Typed PR: DefinitelyTyped/DefinitelyTyped#55123

I've never contributed to this repo before and I made these changes manually. I tried running yarn run docs and it removed all of my changes. What do I need to do to make this change mergeable?

@chriskrycho
Copy link
Contributor

@stefanpenner thoughts on merging this and then fixing the yarn run docs side of things?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants