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 Mouse and Keyboard interface to return Promises #45

Merged
merged 4 commits into from
May 24, 2024

Conversation

allansson
Copy link

Please fill in this template.

Copy link
Author

@allansson allansson left a comment

Choose a reason for hiding this comment

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

Self-review

Copy link
Author

@allansson allansson May 23, 2024

Choose a reason for hiding this comment

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

I tried to find usages in @example tags for all the changes, but I couldn't find any. I figure that I can update them if I find them in subsequent PRs.

Copy link
Member

Choose a reason for hiding this comment

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

No worries. There are some usage examples here and here that might help 🤞

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I could have been more clear. I meant that some JSDoc comments include @examples tags and those need to be updated with await, but I couldn't find any using Cmd + F.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see. I believe you mean none of the examples have a mouse and keyboard example in their JSDoc comments. It's OK. On a tangent, do you think it would be better if we added some example usages to the mouse and keyboard JSDoc comments (not necessary to do it ATM, but nice-to-have)?

Copy link
Author

Choose a reason for hiding this comment

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

This is where I wish we could generate reference docs from the type definitions instead, because having examples in both places would mean us having to keep examples in sync. If the type definitions were the source of truth for the docs, you'd have the same docs in the IDE and on the web.

Copy link
Member

Choose a reason for hiding this comment

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

That would be really useful, so we won't have to type k6-docs manually and keep everything in sync.

However, k6-docs examples are more involved, while typescript examples, on the other hand, are small code snippets (does it make sense to add full-blown examples in the typescript comments? I'm not sure).

Copy link

Choose a reason for hiding this comment

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

This is where I wish we could generate reference docs from the type definitions instead, because having examples in both places would mean us having to keep examples in sync

This would be very cool! Would make things simpler for sure.

I usually like to create the simplest example showcasing a typical use case in the type definitions e.g. keyboard.type("Hello World"). In the docs themselves I'd place a full blown working example that can be copy and pasted and ran without any modifications.

Copy link
Member

@inancgumus inancgumus left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks!

types/k6/experimental/browser.d.ts Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

No worries. There are some usage examples here and here that might help 🤞

Copy link

@ankur22 ankur22 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@allansson allansson merged commit 3c5382f into master-async-browser May 24, 2024
1 check passed
@ankur22 ankur22 deleted the async/mouse-and-keyboard branch May 24, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants