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

fix: highlight issue #3896

Merged
merged 4 commits into from
Oct 9, 2023
Merged

fix: highlight issue #3896

merged 4 commits into from
Oct 9, 2023

Conversation

kobenguyent
Copy link
Collaborator

Motivation/Description of the PR

Applicable helpers:

  • Playwright

Type of change

  • 🐛 Bug fix

Checklist:

  • Tests have been added
  • Documentation has been added (Run npm run docs)
  • Lint checking (Run npm run lint)
  • Local tests are passed (Run npm test)

@cjhille
Copy link
Contributor

cjhille commented Sep 27, 2023

The issue seems resolved using this branch, thank you 🙏
Although i'm still not quite sure if the highlight precondition is right
if (this.options.highlightElement && store.debugMode) {
This means: Even if I set highlightElement : true in conf.js it will not work unless debugMode is enabled, which is not entirely under the users control.
I don't think we should use store.debugMode in this check, as this is changed at runtime by various other methods, which doesn't make sense if you want the user to be able to configure the use of highlighting.
https://github.com/search?q=repo%3Acodeceptjs%2FCodeceptJS+%22debugmode+%3D+true%22&type=code

@kobenguyent
Copy link
Collaborator Author

highlight-false-verbose-false.webm
highlight-false-verbose-true.webm
highlight-true-verbose-false.webm
higlight-true-verbose-true.webm

Attached the videos, the filename is the combination.

vebose/ highlight TRUE FALSE
TRUE x x
FALSE x x

@kobenguyent kobenguyent merged commit 128274f into 3.x Oct 9, 2023
8 checks passed
@kobenguyent kobenguyent deleted the fix-highlight-issue branch October 9, 2023 08:24
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.

Issues with tryTo and highlightElement using Playwright
4 participants