Skip to content

(webdriverio): fix isDisplayed for Firefox, updated examples in docs#11681

Merged
christian-bromann merged 3 commits intowebdriverio:mainfrom
erwinheitzman:fix-isdisplayed-issues
Nov 20, 2023
Merged

(webdriverio): fix isDisplayed for Firefox, updated examples in docs#11681
christian-bromann merged 3 commits intowebdriverio:mainfrom
erwinheitzman:fix-isdisplayed-issues

Conversation

@erwinheitzman
Copy link
Member

Proposed changes

Fixes #11563

Types of changes

  • Bugfix (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 not work as expected)
  • Documentation update

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • [] I have added necessary documentation (if appropriate)
  • I have added proper type definitions for new commands (if appropriate)

Reviewers: @webdriverio/project-committers

return false
}

const isListedBrowser = noW3CEndpoint.includes((browser.capabilities as WebdriverIO.Capabilities).browserName?.toLowerCase()!) ||
Copy link
Member

Choose a reason for hiding this comment

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

Let's just use the atom for all cases and remove the complexity it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Doing this, does mean that we no longer support the old Microsoft Edge (non Chromium), that is fine for me but I do want to make sure we are aware.

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "not supporting it"? It would just use the atom to determine the visibility which hopefully results in the same. That said though, I don't think anyone uses this browser for testing either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I don't think anyone uses it either and looking again I think I might have been mistaken

@christian-bromann
Copy link
Member

The tests seem failing because we only mock the isElementDisplayed endpoint while the browser actually calls execute which the atom script. We need to adjust this here:

isEventuallyDisplayedScenario() {
this.nockReset()
const elemResponse = { [ELEM_PROP]: ELEMENT_ID }
this._mock.command.findElement().times(1).reply(404, NO_SUCH_ELEMENT)
this._mock.command.findElement().times(2).reply(200, { value: elemResponse })
this._mock.command.isElementDisplayed(ELEMENT_ID).once().reply(200, { value: true })
}

@erwinheitzman erwinheitzman marked this pull request as ready for review November 17, 2023 17:27
@erwinheitzman
Copy link
Member Author

I'm currently at a dinner, will fix the remaining tests once I am back 👍

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Bug Fix 🐛 PRs that contain bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: 'isDisplayed' API works incorrectly in chrome for elements with zero opacity or not in viewport

2 participants