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 Shouldn't process shadow ui elements (close #1570) #1571

Merged
merged 2 commits into from
Apr 17, 2018

Conversation

LavrovArtem
Copy link
Contributor

No description provided.

@LavrovArtem LavrovArtem requested a review from miherlosev April 16, 2018 12:43
@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit a0436bc have failed. See details:

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit bcebd87 have failed. See details:

@LavrovArtem
Copy link
Contributor Author

@testcafe-build-bot \retest

@testcafe-build-bot
Copy link
Collaborator

✅ Tests for the commit bcebd87 have passed. See details:

@@ -46,7 +46,7 @@ export default class NodeSandbox extends SandboxBase {
_processElement (el) {
const processedContext = el[INTERNAL_PROPS.processedContext];

if (processedContext !== this.window) {
if (!domUtils.isShadowUIElement(el) && processedContext !== this.window) {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor enhancement: let's use a fast forward paradigm and remove one level of the brackets

...
...
if (domUtils.isShadowUIElement(el) || processedContext === this.window)
   retrun;
...

test('should not process shadow ui elements (GH-1570)', function () {
var shadowUIElements = hammerhead.nativeMethods.querySelectorAll.call(document, '[class$="-hammerhead-shadow-ui"]');

for (var i = 0; i < shadowUIElements.length; i++)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also before the loop, add a check that garanties there are shadow ui elements.

ok(shadowUIElements.length);

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit cabb1fc have failed. See details:

@LavrovArtem
Copy link
Contributor Author

@testcafe-build-bot \retest

@testcafe-build-bot
Copy link
Collaborator

✅ Tests for the commit cabb1fc have passed. See details:

@LavrovArtem
Copy link
Contributor Author

FPR

@miherlosev miherlosev merged commit 407d274 into DevExpress:master Apr 17, 2018
@LavrovArtem LavrovArtem deleted the i1570 branch April 17, 2018 11:12
AndreyBelym pushed a commit to AndreyBelym/testcafe-hammerhead that referenced this pull request Feb 28, 2019
…evExpress#1571)

* fix `Shouldn't process shadow ui elements` (close DevExpress#1570)

* fix review issues
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.

3 participants