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

Add isVisible and stub transition/transition-group by default #212

Merged
merged 7 commits into from
Sep 26, 2020

Conversation

lmiller1990
Copy link
Member

@lmiller1990 lmiller1990 commented Sep 25, 2020

resolves #210

  • Add isVisible back. We decided to un-deprecate it in V1, porting it to V2.
  • We stub transition and transition group by default in V1. We will do the same here for consistency.
  • This solves the problem in Testing for visibility with transition (v-show) #210 (see there for more context)
  • Stubbing transitions avoids the problem where transitions take time (eg 0.25s) and your assertion runs before your transition has finished. In unit tests you don't really care about things like CSS transitions; you just care if things are visible or not via v-show.
  • You can opt out on a test-by-test basis by passing transition: false and transition-group: false to global.stubs.
  • you can opt out globally by doing config.global.stubs = { transition: false }. This is useful for libraries like cypress-vue who do not stub things, since they are not constrained by things like nextTick like when testing in jsdom with jest.

@lmiller1990 lmiller1990 changed the title WIP Add isVisible and stub transition/transition-group by default Add isVisible and stub transition/transition-group by default Sep 25, 2020
}
}

it('returns false when element hidden via v-show', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: WDYT of adding one more pair of tests returns false when ancestor is hidden via v-show to capture our intent - we're detecting visibility not only on entire element itself

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

Copy link
Member

@cexbrayat cexbrayat left a comment

Choose a reason for hiding this comment

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

LGTM, nice work @lmiller1990 !

element.nodeName !== '#comment' &&
isStyleVisible(element) &&
isAttributeVisible(element) &&
(!element.parentElement || isElementVisible(element.parentElement))
Copy link
Member

Choose a reason for hiding this comment

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

I guess this was battle-tested in VTU 1, but just in case here is how jQuery tests the visibility https://github.com/jquery/jquery/blob/d0ce00cdfa680f1f0c38460bc51ea14079ae8b07/src/css/hiddenVisibleSelectors.js (we implement the same logic in an Angular testing library).

Copy link
Member Author

@lmiller1990 lmiller1990 Sep 26, 2020

Choose a reason for hiding this comment

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

I just coped this from another library (dom-testing-library, or something? same as V1).

There is even more robust visible checks out there; since the primary use case is v-show, which does this via display: none, I think this should cover the main use cases. We could make it more robust if someone has a problem. Surprisingly how had a true visibility selector can be to implement...

Copy link
Member

Choose a reason for hiding this comment

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

@lmiller1990 lmiller1990 merged commit db9fb9a into master Sep 26, 2020
@lmiller1990 lmiller1990 deleted the issue-210-is-visible branch September 26, 2020 09:07
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.

Testing for visibility with transition (v-show)
4 participants