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.

Sign up for GitHub

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

Jump to bottom

Make findTargets support a this.target of type Element #246

Merged
merged 6 commits into from
Feb 2, 2020
Merged

Make findTargets support a this.target of type Element #246

merged 6 commits into from
Feb 2, 2020

Conversation

dwilhelmi
Copy link
Contributor

@dwilhelmi dwilhelmi commented Jan 14, 2019 •
edited by Turbo87
Loading

Resolves #245

@dwilhelmi dwilhelmi mentioned this pull request Jan 14, 2019
Closed
Turbo87
Turbo87 reviewed Jan 15, 2019
View reviewed changes
lib/assertions/exists.ts Outdated Show resolved Hide resolved
lib/assertions.ts Show resolved Hide resolved
@scalvert
Copy link
Collaborator

scalvert commented Oct 3, 2019

Any chance we can dust off this PR? It looks like there may be some straightforward changes to make.

@scalvert scalvert force-pushed the support-Element-type-for-exists branch from 8fe0b2f to 4b9b4ec Compare October 6, 2019 20:56
@scalvert
Copy link
Collaborator

scalvert commented Oct 6, 2019

@Turbo87 in light of this PR, thoughts on changing all references to Element instead of HTMLElement? It feels like that base class is more accurate to what the code is using.

@scalvert
Copy link
Collaborator

Once CI passes this is GTG.

@scalvert
Copy link
Collaborator

I'll follow up this PR with one that renames all references to HTMLElement to Element, so that we're correctly reflecting the type in the comments.

@scalvert scalvert requested a review from Turbo87 October 29, 2019 00:54
@scalvert
Copy link
Collaborator

scalvert commented Nov 6, 2019

@Turbo87 do you have an concerns with me merging this. Do you need to get 👀 on it? I feel as if it's mostly additive.

@alexhancock
Copy link

New to the conversation, but I would like to see this merged as well

Turbo87
Turbo87 reviewed Nov 16, 2019
View reviewed changes
lib/__tests__/is-visible.ts Outdated Show resolved Hide resolved
lib/__tests__/is-visible.ts Outdated Show resolved Hide resolved
lib/assertions.ts Outdated Show resolved Hide resolved
tests/acceptance/qunit-dom-test.js Outdated Show resolved Hide resolved
@Turbo87 Turbo87 force-pushed the support-Element-type-for-exists branch from 29cade3 to b117d1e Compare February 2, 2020 13:39
@Turbo87 Turbo87 merged commit 2bdb294 into mainmatter:master Feb 2, 2020
@Turbo87 Turbo87 added the bug label Feb 2, 2020
@snyk-bot snyk-bot mentioned this pull request Mar 25, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Turbo87 Turbo87 Turbo87 left review comments

@scalvert scalvert scalvert left review comments

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

assert.dom(Element).exists() throws even if it exists
4 participants
@dwilhelmi @scalvert @alexhancock @Turbo87