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

[wdspec] doc adding new WebDriver BiDi modules #49070

Merged
merged 5 commits into from
Nov 25, 2024
Merged

Conversation

sadym-chromium
Copy link
Contributor

@sadym-chromium sadym-chromium commented Nov 8, 2024

Explain adding WebDriver BiDi modules in example of Permissions.

@wpt-pr-bot wpt-pr-bot added the docs label Nov 8, 2024
@sadym-chromium sadym-chromium changed the title [wdspec] add bluetooth module [wdspec] doc adding new modules Nov 8, 2024
@sadym-chromium sadym-chromium requested review from OrKoN and jcscottiii and removed request for sideshowbarker November 8, 2024 17:05
@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you!

##### Create `BidiModule`

BiDi modules are defined in the `tools/webdriver/webdriver/bidi/modules/` directory.
To add a new module called `bluetooth`, declare a Python class
Copy link
Contributor

Choose a reason for hiding this comment

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

I would quite strongly prefer the examples here to be something that has multi-vendor support.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would agree here. The Permissions API can be used instead for such an example. It's one which has cross-browser support.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure this guide requires a specific module as an example, I would be happy with a foo module using a bar command but permissions sounds good to me too.

Copy link
Contributor Author

@sadym-chromium sadym-chromium Nov 11, 2024

Choose a reason for hiding this comment

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

Switched to Permissions, as it allows to provide a specific code snippets comparing to an abstract foo.

Copy link
Contributor Author

@sadym-chromium sadym-chromium Nov 13, 2024

Choose a reason for hiding this comment

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

@jgraham was it the only blocker for this doc?

##### Create `BidiModule`

BiDi modules are defined in the `tools/webdriver/webdriver/bidi/modules/` directory.
To add a new module called `bluetooth`, declare a Python class
Copy link
Contributor

Choose a reason for hiding this comment

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

I would agree here. The Permissions API can be used instead for such an example. It's one which has cross-browser support.

docs/writing-tests/wdspec.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jcscottiii jcscottiii left a comment

Choose a reason for hiding this comment

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

Thanks for this. LGTM.

@sadym-chromium sadym-chromium changed the title [wdspec] doc adding new modules [wdspec] doc adding new WebDriver BiDi modules Nov 15, 2024
Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

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

Approved subject to the suggestion or similar being adopted.

docs/writing-tests/wdspec.md Outdated Show resolved Hide resolved
@sadym-chromium sadym-chromium enabled auto-merge (squash) November 25, 2024 11:24
@sadym-chromium sadym-chromium enabled auto-merge (squash) November 25, 2024 11:24
@sadym-chromium sadym-chromium merged commit 8407637 into master Nov 25, 2024
11 checks passed
@sadym-chromium sadym-chromium deleted the sadym/bidi-doc branch November 25, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants