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

Conflict between Clear event and Save event #306

Open
saeednazarix opened this issue Nov 19, 2021 · 3 comments
Open

Conflict between Clear event and Save event #306

saeednazarix opened this issue Nov 19, 2021 · 3 comments
Labels
improvement Something could be improved question Further information is requested

Comments

@saeednazarix
Copy link

saeednazarix commented Nov 19, 2021

What is the current behavior?

why click on the clear button would fire save event? It creates unwanted behavior.
also related to this:
if we use color on save event something like this:

color.toRGBA().toString(0)

when clicking on the clear button it would throw an error in console.log because after clicking on the clear button it would fire save event and because the color is null it would throw an error in this situation and every code in the clear event gets ignored and won't run.

Please provide the steps to reproduce and create a JSFiddle.

in this example, I added an alert to "save event". after clicking on the clear button, an alert is shown. it is an undesired behavior

  .on('save', (color, instance) => {
    alert()
  }).on('clear', instance => {
  })

Please provide the steps to reproduce and create a JSFiddle.

Click on clear button would throw an error because the clear button is calling save but the color is null now.
Meanwhile in this mode, if we have some code on the clear event, they won't run.

  .on('save', (color, instance) => {
    console.log('Event: "save"', color.toRGBA().toString(0));
  }).on('clear', instance => {
    console.log('Event: "clear"');
  })

What is the expected behavior?

clear button should not fire save event. we may encounter other problems later.
also if we use color.toRGBA() on save event it should not throw an error after clicking on clear button, also codes on the clear event in this situation should run.
temporary fix to this issue is using instance._color.toRGBA() on save event if we want to use the clear button as well. but firing save event is still the main problem.

Your environment:

Version (see Pickr.version): latest - 1.8.2
Used bundle (es5 or normal one): both
Used theme (default is classic): all themes
Browser-version:  chrome 96 / firefox 94 / safari 15
Operating-system:  macOS Monterey 12.0.1
@saeednazarix saeednazarix added the unconfirmed The problem isn't really clear label Nov 19, 2021
@saeednazarix
Copy link
Author

sorry, I didn't realize some of my sentences were removed on this topic. I updated sentences in "What is the current behavior?"

@simonwep
Copy link
Owner

Hey :) Thank your for investing some time into this issue, the reason behind fireing the save event with null on clear is that you're technically changing the color (e.g. saving something) which in this case is "nothing" e.g. null. Since this'd be a breaking change and this project is about to get deprecated in a month I'll consider this an improvement and mark it as such.

@simonwep simonwep added improvement Something could be improved question Further information is requested and removed unconfirmed The problem isn't really clear labels Nov 20, 2021
@Kazem-ma79
Copy link

Hey :) Thank your for investing some time into this issue, the reason behind fireing the save event with null on clear is that you're technically changing the color (e.g. saving something) which in this case is "nothing" e.g. null. Since this'd be a breaking change and this project is about to get deprecated in a month I'll consider this an improvement and mark it as such.

Having the same problem please fix this before archiving project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Something could be improved question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants