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

[Bug]: ColorPicker does not accept empty color #5905

Closed
cpiber opened this issue Dec 18, 2024 · 8 comments
Closed

[Bug]: ColorPicker does not accept empty color #5905

cpiber opened this issue Dec 18, 2024 · 8 comments
Assignees
Labels
Type: Bug 🐞 Something isn't working
Milestone

Comments

@cpiber
Copy link

cpiber commented Dec 18, 2024

Blazorise Version

1.7

What Blazorise provider are you running on?

None

Link to minimal reproduction or a simple code snippet

<ColorPicker Color="null" />

Steps to reproduce & bug description

Set Color property to null

What is expected?

The item should be set to no color, same as when the "Clear" button is used

What is actually happening?

Black is selected

What browsers do you see the problem on?

No response

Any additional comments?

I have a strong suspicion it is because of this line:

default: options.default || "#000000",

The JS part has a fallback again; the blazor part already sets this default, so this should not be there

@cpiber cpiber added the Type: Bug 🐞 Something isn't working label Dec 18, 2024
@stsrki stsrki self-assigned this Dec 18, 2024
@stsrki
Copy link
Collaborator

stsrki commented Dec 18, 2024

There seem to be two problems here.

  1. As you have assumed the default: options.default || "#000000" is more corrected to be defined as default: options.default || "#00000000". Which will set the color as transparent, same as it is in preview.
  2. The <ColorPicker Color="null" /> is not correct to define it as null. Remember, the Color parameter is a string type, so in this case, you have defined it as string "null" value. The expected value format should be a HEX. That means to really to define it as a null, it needs to be defined as <ColorPicker Color="@((string)null)" />.

We will fix the first issue.

@cpiber
Copy link
Author

cpiber commented Dec 18, 2024

is more corrected to be defined as default: options.default || "#00000000"

That still is not correct, is it? Clicking "Clear" sets Color to null, so giving null should also work without changing the value

The is not correct to define it as null. Remember, the Color parameter is a string type, so in this case, you have defined it as string "null" value. The expected value format should be a HEX. That means to really to define it as a null, it needs to be defined as .

null is equivalent to @((string)null) because string is nullable

@stsrki
Copy link
Collaborator

stsrki commented Dec 18, 2024

null is equivalent to @((string)null) because string is nullable

No. The .razor engine is kinda to blame here. When it tries to solve what you define for the string parameter it takes it as it is written. In your case, the actual value for Color="null" when inspected in the debugger would be the "null", not the null.

@stsrki
Copy link
Collaborator

stsrki commented Dec 18, 2024

That still is not correct, is it? Clicking "Clear" sets Color to null, so giving null should also work without changing the value

This problem is from the original library. The real color cannot be undefined. So we try to have default color that makes some sense.

See simonwep/pickr#178

@cpiber
Copy link
Author

cpiber commented Dec 18, 2024

Interesting, good to know. In my actual test I have a variable that is null and it behaves the same, so I did not notice, thanks for the info.
Still, the input should not change the color to anything without the user selecting something, and both "#000000" and "#00000000" are different from null

@cpiber
Copy link
Author

cpiber commented Dec 18, 2024

This problem is from the original library. The real color cannot be undefined. So we try to have default color that makes some sense.

I see. The author provides a workaround in simonwep/pickr#178 (comment), would that be acceptable? I can look into implementing it

@stsrki
Copy link
Collaborator

stsrki commented Dec 18, 2024

That might work. Working on it.

@stsrki stsrki added this to Support Dec 18, 2024
@github-project-automation github-project-automation bot moved this to 🔙 Backlog in Support Dec 18, 2024
@stsrki stsrki added this to the 1.7 support milestone Dec 18, 2024
@stsrki stsrki closed this as completed Dec 18, 2024
@github-project-automation github-project-automation bot moved this from 🔙 Backlog to ✔ Done in Support Dec 18, 2024
@cpiber
Copy link
Author

cpiber commented Dec 18, 2024

Thank you for the fast work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐞 Something isn't working
Projects
Status: ✔ Done
Development

No branches or pull requests

2 participants