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

[πŸš€ Feature]: Convenience methods for BiDi #13991

Closed
titusfortner opened this issue May 20, 2024 · 9 comments
Closed

[πŸš€ Feature]: Convenience methods for BiDi #13991

titusfortner opened this issue May 20, 2024 · 9 comments
Labels
A-needs decision C-devtools BiDi or Chrome DevTools related issues I-enhancement
Milestone

Comments

@titusfortner
Copy link
Member

titusfortner commented May 20, 2024

Feature and motivation

Java & Python are missing convenience methods for enabling BiDi.

Right now we have....
.NET β€” options.UseWebSocketUrl = true
Ruby β€” options.web_socket_url = true
Python β€” options.enable_bidi = True
JS β€” enableBidi()
Java - enableBidi()

Should we update everything to use enableBiDi() name?

Usage example

options.enableBiDi(true)
@titusfortner
Copy link
Member Author

@pujagani I'm guessing we don't have a good reason for not adding it to java, yet?

@pujagani
Copy link
Contributor

Thank you for bringing this up. Actually we can add it.

@pujagani pujagani added the C-devtools BiDi or Chrome DevTools related issues label May 22, 2024
@pujagani
Copy link
Contributor

"options.enableBiDi(true)" - @titusfortner Should the user need to pass in true? Isn't enableBidi() sufficient?

pujagani added a commit to pujagani/selenium that referenced this issue May 22, 2024
@titusfortner
Copy link
Member Author

titusfortner commented May 22, 2024

@pujagani I thought the other methods in Java Options that are boolean require passing in true/false.
So, we should do whatever matches the rest of our code.

@pujagani
Copy link
Contributor

pujagani commented May 23, 2024

That is a fair point. Though the naming, in that case, is like setEnableDownloads(boolean enableDownloads). I was thinking if the name is enableBiDi, then that means just enable it. But if we want to set a value then the name should be such. What do you think?

@titusfortner
Copy link
Member Author

If setXX(boolean) is the pattern, then we should follow that, though I agree it seems a bit much. @diemol any opinions on our conventions here?

@diemol
Copy link
Member

diemol commented May 23, 2024

I am not sure. If we want to set a property or a value, I like the setXYZ pattern, but if we want to enable a feature, I prefer the enableXYZ pattern.

This is to be said, I saw setEnableDownloads too late; it was already released. I would have done enableDownloads.

@pujagani
Copy link
Contributor

Thank you folks. I think I will go with enableBiDi.

pujagani added a commit to pujagani/selenium that referenced this issue May 24, 2024
diemol pushed a commit that referenced this issue May 24, 2024
@diemol diemol removed the C-java label May 24, 2024
@diemol diemol modified the milestones: 4.22, 4.23 Jun 21, 2024
@pujagani
Copy link
Contributor

Present in python a51ddee

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-needs decision C-devtools BiDi or Chrome DevTools related issues I-enhancement
Projects
None yet
Development

No branches or pull requests

3 participants