Skip to content

Conversation

@iOrangeTea
Copy link

@iOrangeTea iOrangeTea commented Feb 10, 2025

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement


Description

  • Added support for tap events in Selenium's JavaScript atoms.

  • Introduced tap function in webdriver.atoms.inputs for touchscreen interactions.

  • Updated BUILD.bazel to include a new closure fragment for tap.

  • Modified events.js to handle initTouchEvent differently for iOS.


Changes walkthrough 📝

Relevant files
Enhancement
events.js
Update `initTouchEvent` handling for iOS compatibility     

javascript/atoms/events.js

  • Adjusted initTouchEvent handling for non-iOS devices.
  • Commented out the previous length check for initTouchEvent.
  • +2/-1     
    inputs.js
    Export `tap` function in `webdriver.atoms.inputs`               

    javascript/webdriver/atoms/exports/inputs.js

    • Exported the new tap function for external use.
    +2/-0     
    action.js
    Add `tap` function to inject tap actions                                 

    javascript/webdriver/atoms/inject/action.js

  • Added a tap function to inject tap actions on elements.
  • Documented the tap function with JSDoc comments.
  • +13/-0   
    inputs.js
    Add `tap` function for touchscreen interactions                   

    javascript/webdriver/atoms/inputs.js

  • Introduced a tap function for touchscreen interactions.
  • Required bot.Touchscreen for touch-based actions.
  • +13/-0   
    Configuration changes
    BUILD.bazel
    Add closure fragment for `tap` in BUILD.bazel                       

    javascript/webdriver/atoms/inject/BUILD.bazel

  • Added a new closure fragment for the tap function.
  • Linked tap to the action module dependencies.
  • +9/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @CLAassistant
    Copy link

    CLAassistant commented Feb 10, 2025

    CLA assistant check
    All committers have signed the CLA.

    @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Message

    The error message "No element to send keys to" in the tap function is incorrect and misleading since the function is for tapping, not sending keys

    throw Error('No element to send keys to');
    Browser Detection

    The change from checking initTouchEvent.length to checking !IOS may not handle all browser implementations correctly and could break touch event initialization on some browsers

    if (!goog.userAgent.product.IOS) {

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Feb 10, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Improve touch event browser compatibility

    Using !goog.userAgent.product.IOS as condition could cause issues on other
    mobile platforms. Consider keeping the original implementation that checks
    initTouchEvent.length as it's more reliable.

    javascript/atoms/events.js [526-527]

    -// if (event.initTouchEvent.length == 0) {
    -if (!goog.userAgent.product.IOS) {
    +if (event.initTouchEvent.length == 0) {
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies a potential compatibility issue. Using feature detection (checking initTouchEvent.length) is more reliable than platform-specific checks for handling touch events across different mobile browsers.

    High
    General
    Fix incorrect error message text

    The error message in the tap function incorrectly refers to "sending keys"
    instead of "tapping". Update the error message to accurately reflect the tap
    action.

    javascript/webdriver/atoms/inputs.js [87-89]

     if (!element) {
    -  throw Error('No element to send keys to');
    +  throw Error('No element to tap');
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: The error message is misleading as it refers to "sending keys" instead of "tapping". Using the correct error message improves code clarity and debugging experience.

    Medium
    Learned
    best practice
    Move null validation check to start of function and improve error message to include actual value for better debugging

    The tap function should validate the element parameter at the start of the
    function, before creating the touchScreen instance, to fail fast and provide a
    more descriptive error message that includes the actual value.

    javascript/webdriver/atoms/inputs.js [85-91]

     webdriver.atoms.inputs.tap = function(element) {
    +  if (!element) {
    +    throw Error(`Cannot tap on element: element is ${element}`);
    +  }
       var touchScreen = new bot.Touchscreen();
    -  if (!element) {
    -    throw Error('No element to send keys to');
    -  }
       bot.action.tap(element, null, touchScreen);
     };
    • Apply this suggestion
    Low

    @diemol
    Copy link
    Member

    diemol commented Aug 16, 2025

    @iOrangeTea could you please share some context about this PR and sign the CLA?

    @iOrangeTea
    Copy link
    Author

    Hello @diemol, I have already signed the CLA. Thanks for your remind.
    Some web pages of touchable devices use TOUCH_EVENT to trigger clicks instead of MOUSE_EVENTS. For example, the mini-program page of wechat on iOS. So I extended a tap API in the JS version.

    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.

    3 participants