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

Change "!" to "#ffffff" as default color/value #292

Closed
wants to merge 1 commit into from
Closed

Change "!" to "#ffffff" as default color/value #292

wants to merge 1 commit into from

Conversation

wolffe
Copy link

@wolffe wolffe commented Mar 9, 2015

Change "!" to "#ffffff" as default color/value to get rid of the "The specified value '!' does not conform to the required format." error.

Change "!" to "#ffffff" as default color/value to get rid of the "The specified value '!' does not conform to the required format." error.
@bgrins
Copy link
Owner

bgrins commented Mar 9, 2015

IIRC we made it an invalid color on purpose, since any browser that supported input [type=color] would not allow it to be set to that value. Which browser are you using? And is it throwing an exception?

@wolffe
Copy link
Author

wolffe commented Mar 9, 2015

I'm using the latest version of Chrome as of today and I get a console warning (not an error). The only browser that doesn't have support for input [type=color] is Safari and it shows a text input only. I don't think the "!" value is necessary.

@bgrins
Copy link
Owner

bgrins commented Mar 9, 2015

If we use a valid color string, then the check would not be useful at all since all browsers will return that value regardless of whether they support color inputs. Any browser that doesn't support it will fall back to a text input, but we still need to know which these are.

There may be a better way to detect this, but it looks like Modernizr does something similar: https://github.com/Modernizr/Modernizr/blob/7d19f067ddce41450d0684b14003f4c878dc4c66/feature-detects/inputtypes.js#L61.

@wolffe
Copy link
Author

wolffe commented Mar 9, 2015

I understand. For my purposes, I won't bloat my code with Modernizr and I will fallback Safari users to a text value. I'm using spectrum.js on a graphic design community, so they should be familiar with RGB hex codes.

@bgrins
Copy link
Owner

bgrins commented Mar 9, 2015

Are you ever wanting to fall back to the native input[type=color]? Because if not and you don't care about the native control then you could just set inputTypeColorSupport = true in the code and then just never initialize spectrum with an input[type=color].

@wolffe
Copy link
Author

wolffe commented Mar 9, 2015

Thanks for that, I don't want to fall back to the native support as most of my user base use Safari. Didn't think of hardcoding the check. Will do and also keep an eye on your changes.

You could add an option (detect: true/false) so I can use "true" and still use your version, not my forked one.

@isaacdd
Copy link

isaacdd commented Mar 20, 2015

+1 for adding an option for the default value.

@bgrins
Copy link
Owner

bgrins commented Mar 21, 2015

OK, this would be a reasonable way to handle it:

  • Make inputTypeColorSupport a function instead of a variable (it can still cache the result of the calculation so it doesn't warn every time the function is called)
  • Move the call to inputTypeColorSupport() to the end of the conditions here:
    isInputTypeColor = isInput && inputTypeColorSupport && boundElement.attr("type") === "color",
  • On load, move the call to inputTypeColorSupport() to after we figure out if there is actually any `input[type=color]" here:
    if (!inputTypeColorSupport) {

@bgrins
Copy link
Owner

bgrins commented Mar 21, 2015

I'll file a PR to get rid of the warning when no input type="color" elements are passed in

@bgrins
Copy link
Owner

bgrins commented Mar 21, 2015

This is fixed by #296.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants