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

Active window watching mechanism implemented #49

Closed

Conversation

helen-dikareva
Copy link
Collaborator

It's necessary to fix the T235186 TestCafe bug

/cc @miherlosev @LavrovArtem @inikulin

@VasilyStrelyaev please check the note in the src/client/sandboxes/event/active-window.js file

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 0763a13 have failed. See details.

@testcafe-build-bot
Copy link
Collaborator

✅ Tests for the commit 83fd28b have passed. See details.

@VasilyStrelyaev
Copy link
Collaborator

lgtm

@helen-dikareva
Copy link
Collaborator Author

FPR

@miherlosev
Copy link
Contributor

I think we need tests that cover the following scenarious:
1)initially, the iframe №1 window is active, then activate the iframe №2 window
2)initially, the iframe window is active, then activate the top window by clicking on shadow ui element.

@AlexanderMoskovkin
Copy link
Contributor

I think we need tests that cover the following scenarious:
initially active is iframe...

The top window is active always while the page is loading and our scripts initializing. What the reason of testing scenarios when an iframe is active at test start?

(activate by clicking on shadow ui element)

This tests - unit tests for the active-window module. The Shadow UI has no relation to this

@AlexanderMoskovkin
Copy link
Contributor

ok, we have discussed it personally. We'll add two tests:

  • for the active-window module to test the scenario focus iframe1 -> focus iframe2
  • for the focus-blur module to check that activeWindow.set is not called when an iframe is focused and focus called for the shadow ui element

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 150f1f0 have failed. See details.

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 67da233 have failed. See details.

}
}

export function init () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we have init() function I think it should be better to call this ActiveWindowTracker and make it class instead of set of functions (moreover it has internal state)

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 396c15f have failed. See details.

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit ac0293f have failed. See details.

@testcafe-build-bot
Copy link
Collaborator

Tests for the commit ee058d7 have started. See details.

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 224a472 have failed. See details.

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit ee058d7 have failed. See details.

@inikulin
Copy link
Contributor

What's going on here?

@helen-dikareva
Copy link
Collaborator Author

PR closed due to wrong merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants