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

use CSS queries #718

Merged
merged 6 commits into from
Oct 30, 2019
Merged

use CSS queries #718

merged 6 commits into from
Oct 30, 2019

Conversation

tinovyatkin
Copy link
Contributor

@tinovyatkin tinovyatkin commented Oct 30, 2019

Two small changes related to more effective use of CSS queries in JS:

  1. use window.matchMedia('min-height') instead of (incorrect) window height calculation (supersedes Updated windowHeight to be based on viewport #633, sorry @justin-duncan 🤷🏼‍♂️)
  2. use last-of-type instead of querySelectorAll().pop 😮

And one change to SCSS to use visibility: hidden instead of display: none to prevent one extra reflow on reveal.

@tinovyatkin tinovyatkin changed the title use last-of-type use CSS queries Oct 30, 2019
@justin-duncan
Copy link

Downloaded your fork and built it, but it doesn't solve what my issue was that it doesn't flip based on current view-port, now it seems like the default is always up, am I missing something?

@jshjohnson jshjohnson added housekeeping Pull request that improves code readability but maintains behaviour refactor Pull request that refactors existing functionality and removed housekeeping Pull request that improves code readability but maintains behaviour labels Oct 30, 2019
@tinovyatkin
Copy link
Contributor Author

@justin-duncan you are right, sorry, seems like need to remove more code here.
Is that looks right now?

Screenshot 2019-10-30 at 09 02 55

Screenshot 2019-10-30 at 09 02 43

@justin-duncan
Copy link

Yeah, that's much better if its more efficient, appreciated @tinovyatkin.

@tinovyatkin
Copy link
Contributor Author

tinovyatkin commented Oct 30, 2019

@justin-duncan it's more efficient at least due to fact that it doesn't causes extra reflow(s) here. And way less code, until @jshjohnson knows some side effect these code had.

@jshjohnson
Copy link
Collaborator

This works really nicely 👍 I can't stand dealing with window/document heights and widths in javascript...

@jshjohnson jshjohnson merged commit 034191c into Choices-js:master Oct 30, 2019
@tinovyatkin tinovyatkin deleted the match-media branch October 30, 2019 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Pull request that refactors existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants