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

Fix hidden pickr instances issues #229

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DEfusion
Copy link
Contributor

@DEfusion DEfusion commented Jul 7, 2020

If the container the pickr is output in is not visible when it is created the position of the movable handles is not shown correctly on opening, this then has the knock-on effect of not setting the color correctly on interaction unless you click on the palette or a swatch first.

This change addresses that by ensuring the color is set on the first call to show after the picker is made visible, this also reverts the changes from 0d445e5.

Fixes #226

If the container the pickr is output in is not visible when it is created the position of the movable handles is not shown correctly on opening, this then has the knock-on effect of not setting the color correctly on interaction unless you click on the palette or a swatch first.

This change addresses that by ensuring the color is set on the first run of `show` after the picker is made visible.

Fixes simonwep#226
@simonwep
Copy link
Owner

Hmm, I wonder if we could just move the entire initialization code to when show gets called the first time. I'm not sure how that would react to different environments though...

I'm also not certain about this as the default color-value is more or less ignored, what if show get's called first (the parent is still invisible) but the default color hasn't been assigned yet 🤔

@DEfusion
Copy link
Contributor Author

Yeah I'm not sure if moving everything to show would work for every use case as not as familiar with other uses than our use case, it would be nice to get rid of the infinitely running callback waiting for the picker to visible.

I was wondering about whether the moveables could/should detect that they're not visible and have those handle it somehow because it's there that things break down on the initial update calls.

I can certainly amend this PR to try and move initialization to show or any other ideas, I was worried about the init event not being triggered that may cause problems but that also won't be triggered right now until it's visible anyway...

@simonwep simonwep added the bug Something isn't working label Jul 23, 2020
@simonwep simonwep force-pushed the master branch 2 times, most recently from 3e552aa to c1ebb36 Compare December 5, 2020 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initial color not set correctly when pickr parent is not visible.
2 participants