-
Notifications
You must be signed in to change notification settings - Fork 773
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
chore(VirtualRule): add nodeIndex property #2955
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see a more comprehensive merge-results test that uses more than just a single iframe. Something with multiple iframes and a nested iframe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Could you add one more test for empty array nodeIndex, since that is the state cased by an element not being in the tree? After that i think it's good to merge
* chore(VirtualRule): add nodeIndex property * chore(DqElement): add nodeIndexes * chore(DqElement): fix up fromFrame * chore(dqelement): fix for IE * chore(mergeResult): merge using nodeIndex * chore: formatting * chore: fix iframe order sorting * chore: resolve feedback * chore(merge-results): deal with invalid nodeIndexes * chore: more tests * chore: consistent node sorting * chore: more fun with sorting * fix(merge-results): handle invalid nodeIndexes * fix: nodeIndex can be 0 * Update lib/core/utils/dq-element.js
* chore(VirtualRule): add nodeIndex property * chore(DqElement): add nodeIndexes * chore(DqElement): fix up fromFrame * chore(dqelement): fix for IE * chore(mergeResult): merge using nodeIndex * chore: formatting * chore: fix iframe order sorting * chore: resolve feedback * chore(merge-results): deal with invalid nodeIndexes * chore: more tests * chore: consistent node sorting * chore: more fun with sorting * fix(merge-results): handle invalid nodeIndexes * fix: nodeIndex can be 0 * Update lib/core/utils/dq-element.js
* chore(VirtualRule): add nodeIndex property * chore(DqElement): add nodeIndexes * chore(DqElement): fix up fromFrame * chore(dqelement): fix for IE * chore(mergeResult): merge using nodeIndex * chore: formatting * chore: fix iframe order sorting * chore: resolve feedback * chore(merge-results): deal with invalid nodeIndexes * chore: more tests * chore: consistent node sorting * chore: more fun with sorting * fix(merge-results): handle invalid nodeIndexes * fix: nodeIndex can be 0 * Update lib/core/utils/dq-element.js
Closes issue: #2938