Issue:

I couldn't seem to figure out how to define a palette for use with the jQuery Spectrum widget. I finally did some digging and found that spectrum expects a Javascript array format as show here: https://bgrins.github.io/spectrum/#options-showPalette

It seems that the current code simply dumps whatever string you enter into the palette field.

So instead of if I wanted a palette of...say: White, Black, Red -- I tried to enter "#ffffff, #000000, #ff0000" sans quotes into the text area provided.

Current code produced...

...
palette: ["#ffffff, #000000, #ff0000"]],
...

This prevented spectrum from producing the proper palette in the widget because the Spectrum code actually needs something like....

...
palette: [["#ffffff", "#000000", "#ff0000"]],
...

So I created a simple patch that implements string to array split on the bar ( | ) character to allow for both hex and rgb values (as commas would break rgb values).

So now my textarea would contain "#ffffff|#000000|#ff0000"

CommentFileSizeAuthor
#24 color_field_spectrum_palette_fix-2827198-23.patch1.98 KBrwam
#22 color_field_spectrum_palette_fix-2827198-22.patch1.97 KBrwam
#17 color_field_spectrum_palette_fix-2827198-18.patch2.7 KBmcvjoyner
#16 color_field_spectrum_palette_fix-2827198-16.patch646 bytesrwam
#12 color_field_spectrum_palette_fix-2827198-12.patch2.77 KBfrogdog_tech
#9 color_field_spectrum_palette_fix-2827198-9.patch1.76 KBmcvjoyner
#8 color_field_spectrum_palette_fix-2827198-8.patch0 bytesmcvjoyner
#4 color_field-spectrum_palette_fix-8.x.patch715 bytesfrogdog_tech
color_field-spectrum_palette_fix-8.x.patch650 bytesfrogdog_tech

Comments

squidgle created an issue. See original summary.

frogdog_tech’s picture

frogdog_tech commented

Status: Needs review » Needs work

The last submitted patch, color_field-spectrum_palette_fix-8.x.patch, failed testing.

frogdog_tech’s picture

frogdog_tech commented
StatusFileSize
new color_field-spectrum_palette_fix-8.x.patch715 bytes

new patch against 8.x-2.x-dev and not rc1

frogdog_tech’s picture

frogdog_tech commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 4: color_field-spectrum_palette_fix-8.x.patch, failed testing.

frogdog_tech’s picture

frogdog_tech commented

hmm. something broken with automated testing?

modules/color_field/js/color_field_widget_spectrum.jquery.js: No such file or directory
Patch Failed to apply
mcvjoyner’s picture

mcvjoyner commented
StatusFileSize
new color_field_spectrum_palette_fix-2827198-8.patch0 bytes

Patch wasn't quite working, also wasn't following file naming convention.

(See comment #9, as correct file is there).

I've uploaded a patch that also fixes the JS issue, when clicking the dropdown, and then clicking anywhere on the page BUT the widget dropdown, it results in a JS error, that reports that it's unable to read from a null reference.

The pacth also addresses the 'description' of the palette widget (textarea) on the the 'manage form display' administration site.

Since we changed the strings to split with the pipe character, it should help the user out by mentioning so.

Description is now:
"Selectable color palette to accompany the Spectrum Widget. Separate values with a pipe (|) character. Example: #ff0000|#00aa00'"

mcvjoyner’s picture

mcvjoyner commented
StatusFileSize
new color_field_spectrum_palette_fix-2827198-9.patch1.76 KB

Looks like the patch in #8 didn't upload correctly. I'm trying again (same patch), as comment #9.

frogdog_tech’s picture

frogdog_tech commented

#9 looking good! nice work

Is there any reason you opted to keep the original palette assignment?

palette: [spectrum_settings.palette],

frogdog_tech’s picture

frogdog_tech commented
Version: 8.x-2.0-rc1 » 8.x-2.x-dev

changing the version to 8.x-2.x-dev

frogdog_tech’s picture

frogdog_tech commented
StatusFileSize
new color_field_spectrum_palette_fix-2827198-12.patch2.77 KB

Mods to #9

1. Remove original palette: setting
2. The checkbox (boolean) widget settings didn't seem to be working and were always falling back to defaults. I was unable to activate showInput because showPaletteOnly was stuck on. I evaluate the settings from 1 and 0 to true and false and now the checkbox widget settings are working.

mcvjoyner’s picture

mcvjoyner commented

@squidgle, nope no reason, was just not considered :)

Thanks for additions to the patch! #12 Looks good

frogdog_tech’s picture

frogdog_tech commented
Status: Needs work » Needs review

The last submitted patch, 9: color_field_spectrum_palette_fix-2827198-9.patch, failed testing.

rwam’s picture

rwam commented
StatusFileSize
new color_field_spectrum_palette_fix-2827198-16.patch646 bytes

I really like to set up the palette like described at the example page at http://bgrins.github.io/spectrum/#options-showPalette And I want to use a palette with mulitple rows and my config looks like this:

["#fff","#aaa","#f00","#00f"],
["#414141","#242424","#0a8db9"]

So please find attached a simple patch which works fine for me.

mcvjoyner’s picture

mcvjoyner commented
StatusFileSize
new color_field_spectrum_palette_fix-2827198-18.patch2.7 KB

- Going with tradition here and updating the patch so that files are based on the root of the module itself. (From patch #12).
- Adding in patch #16, cause I think it makes sense, and is pretty clean.
- Updated the help text on the field to reflect and give an example of how the input should look.

Everything seems to work well for me.

mcvjoyner’s picture

mcvjoyner commented
Assigned: Unassigned » mcvjoyner
rwam’s picture

rwam commented

Hi @cheeky-micah,

the patch works fine for me with 8.x-2.x-rc2. Thanks for updating.

Ciao
Ralf

tamnv’s picture

tamnv commented
Status: Needs review » Reviewed & tested by the community

Hi @cheeky-micah,

The patch works well for me, thanks for updating.

slydevil’s picture

slydevil commented

Patch also works for me - thanks!

rwam’s picture

rwam commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new color_field_spectrum_palette_fix-2827198-22.patch1.97 KB

Patch 18 failed on new release candidate 8.x-2.0-rc3 and current 8.x-2.x dev branch. After some investigation I've created a new patch to support spectrum palettes again.

Status: Needs review » Needs work

The last submitted patch, 22: color_field_spectrum_palette_fix-2827198-22.patch, failed testing. View results

rwam’s picture

rwam commented
Status: Needs work » Needs review
StatusFileSize
new color_field_spectrum_palette_fix-2827198-23.patch1.98 KB

Hm, I have no idea why the patch failed to apply, but I've updated the patch to use correct translation method.

Status: Needs review » Needs work

The last submitted patch, 24: color_field_spectrum_palette_fix-2827198-23.patch, failed testing. View results

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
Assigned: mcvjoyner » nickdickinsonwilde
Issue tags: +Needs tests, +Needs update path

This is a very nice change.
But... it will break existing definitions and needs tests.

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
Status: Needs work » Fixed
Issue tags: -Needs tests, -Needs update path

Test and HOOK_post_update_NAME() created.

  • NickWilde committed 46225e4 on 8.x-2.x 
    Issue #2827198 by frogdog_tech, cheeky-micah, rwam, NickWilde: Spectrum...
nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
Assigned: nickdickinsonwilde » Unassigned

(and thanks for doing all the different patch iterations and patience as well as the report)

  • NickWilde committed 0f5fddd on 8.x-2.x 
    Issue #2827198 by frogdog_tech, cheeky-micah, rwam, NickWilde: Spectrum...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

frogdog_tech’s picture

frogdog_tech commented

This still fails for RGB since the row explode uses a comma and will break rgba(value,value,value)?