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

Allow step element to be a HTMLElement #42

Closed
wants to merge 2 commits into from

Conversation

coxy
Copy link

@coxy coxy commented Mar 21, 2018

I would find it useful to be able to pass in an existing DOM node rather than a queryString, so I think this PR should allow for that. The change in demo.js is just to validate it's working correctly.

src/index.js Outdated
@@ -239,7 +239,7 @@ export default class Driver {
this.steps = [];

steps.forEach((step, index) => {
if (!step.element || typeof step.element !== 'string') {
if ((!step.element || !(typeof step.element === 'string' || step.element instanceof HTMLElement))) {
Copy link
Owner

Choose a reason for hiding this comment

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

HTMLElement won't be available in browsers not supporting W3 DOM2 (or IE in other words). Would be better to use nodeType

Copy link
Author

Choose a reason for hiding this comment

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

No worries. I'd tested in IE11+ but don't have access to older IEs at the moment as have no need to support them in projects I'm working on. I've updated the PR with a step.element.nodeType > 0 check - but again haven't been able to test in older IEs.

@FabianWilms
Copy link

Any plans on merging this? This would also help to make this library work with shadow dom.

@kamranahmedse
Copy link
Owner

Added in 523519f

Thanks

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

4 participants