Skip to content

added python example for enabling BiDi #1965

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

Merged
merged 2 commits into from
Sep 26, 2024

Conversation

shbenzer
Copy link
Contributor

@shbenzer shbenzer commented Sep 24, 2024

User description

added basic bidi configuration example for python to index.en.md

Description

added configuration example for python

Motivation and Context

only coding language missing example

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

PR Type

documentation


Description

  • Added a Python example for enabling BiDi in the WebDriver documentation.
  • Corrected the tab header for the Python example to ensure consistency with other language examples.

Changes walkthrough 📝

Relevant files
Documentation
_index.en.md
Add Python BiDi configuration example to documentation     

website_and_docs/content/documentation/webdriver/bidi/_index.en.md

  • Added Python example for enabling BiDi.
  • Corrected the tab header for Python example.
  • +3/-3     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    netlify bot commented Sep 24, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 27a9eda

    @qodo-merge-pro qodo-merge-pro bot added the documentation Improvements or additions to documentation label Sep 24, 2024
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Inconsistent Syntax
    The Python example uses a different syntax (options.enable_bidi = True) compared to other languages which use setCapability or similar methods. This might be confusing for users expecting a consistent API across languages.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use the standard method for setting capabilities in Selenium Python

    Consider using the set_capability() method instead of directly assigning to the
    enable_bidi attribute. This is a more standard way of setting capabilities in
    Selenium Python.

    website_and_docs/content/documentation/webdriver/bidi/_index.en.md [38]

    -options.enable_bidi = True
    +options.set_capability("webSocketUrl", True)
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion to use set_capability() is valid as it aligns with standard practices in Selenium Python, improving code consistency and maintainability.

    9
    Enhancement
    Add a comment to explain the purpose of enabling BiDi

    Consider adding a comment to explain what BiDi is and why it's being enabled. This
    can help users understand the purpose of this configuration.

    website_and_docs/content/documentation/webdriver/bidi/_index.en.md [37-39]

     {{< tab header="Python" >}}
    +# Enable BiDi (Bidirectional) protocol for advanced browser control
     options.enable_bidi = True
     {{% /tab %}}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a comment to explain the purpose of enabling BiDi enhances code readability and helps users understand the configuration, though it is not crucial for functionality.

    7

    💡 Need additional feedback ? start a PR chat

    @harsha509 harsha509 merged commit cb40aec into SeleniumHQ:trunk Sep 26, 2024
    3 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    documentation Improvements or additions to documentation Review effort [1-5]: 1
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants