Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Nanopop update issue when using with Pickr and large screen sizes #231

Closed
alexdiliberto opened this issue Jul 10, 2020 · 6 comments
Closed
Labels
bug Something isn't working

Comments

@alexdiliberto
Copy link

First off, just wanted to say I'm really enjoying your Pickr library.

I do have an issue I'm running into on my app that seems to be Nanopop/positioning related. I wasn't sure which library to raise the issue (Pickr or Nanopop) but I'll try here. I have a minimal reproduction on this Codepen:

https://codepen.io/alexdiliberto/pen/vYLrxyq

You can see the issue if you start resizing your screen larger and larger then it seems to begin throwing off the positioning until it finally fails.

1680px screen width (expected behavior)

Screen Shot 2020-07-10 at 11 51 24 AM

2300px screen width

Screen Shot 2020-07-10 at 11 51 39 AM

2450px screen width

Screen Shot 2020-07-10 at 11 54 45 AM

2600px screen width

Screen Shot 2020-07-10 at 11 51 45 AM

eventually on large screens it seems to fail this conditional and just places the pickr app in the center of the screen

Any assistance would be greatly appreciated, none of the public API's for Pickr helped in this case...I manually set this._recalc (https://github.com/Simonwep/pickr/blob/1.7.1/src/js/pickr.js#L513) to false and that seemed to help by just now allowing the final fail case where it places the pickr app in the center of the screen.

@alexdiliberto alexdiliberto changed the title Nanonpop update issue when using with Pickr and large screen sizes Nanopop update issue when using with Pickr and large screen sizes Jul 10, 2020
@simonwep
Copy link
Owner

Hey thank you :) Yes that's perfect, I should also put a not somewhere in pickr that positioning-related issues should go here as nanopop is used in several deployed apps.

Huh that is weird, I tried it with several resolutions including 3840px and it seems to work, what browser / OS are you using? I also did that with pickr and resizing it heavily but nothing broke :/

@alexdiliberto
Copy link
Author

alexdiliberto commented Jul 13, 2020

@simonwep Hmmmm so you tried using the really large screen resolutions, ok. It's happening for me on the basically all major browsers... latest Chrome (83.0.4103.116), latest Firefox (78.0.2), latest Safari (Version 13.1.1) and latest Edge on both Mac and Windows on multiple computers.

Screen recording: https://recordit.co/AI4IUpR5vS

@simonwep
Copy link
Owner

Okay I managed to reproduce the error using the very same screen resolution you've used, I'll take a look into that :)

@simonwep simonwep transferred this issue from simonwep/nanopop Jul 13, 2020
@simonwep
Copy link
Owner

Okay this is a misconfiguration in pickr. The solution was to update the clipping-container each time pickr gets repositioned. I forgot that this value is static and has to be updated each time e.g. it only recognized the screen-size on first page-load leading to miscalculations.

Fixed in b9efcb7, if you want you can test it using the latest builds from master and if everything works as expected I'll release a patch version ASAP.

@simonwep simonwep added the bug Something isn't working label Jul 13, 2020
@alexdiliberto
Copy link
Author

@simonwep Wow thank you for the quick response. Just checked using master branch in my application and this seems to be working perfectly now. 💯 🥇

@simonwep
Copy link
Owner

Great! Patch available in 1.7.2 :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants