? Failure

User tests: Successful: Unsuccessful:

avatar C-Lodder
C-Lodder
31 May 2017

Summary of Changes

This PR introduces a new vanilla Javascript colour picker and removes both the old jQuery pickers. This is still a WIP but I'm trying to keep B/C as much as possible. It does remove the simplecolours, but if you are currently using a colour form field, it should still work as expected with this.

Testing Instructions

Have a mess around with the picker and also test to see if the picker get initiated with subform fields.

Normal (defaults to HEX mode)

<field name="colour1" type="color" label="Colour" default="#000000" />

RGB Mode

<field name="colour2" type="color" label="Colour" default="rgb(0,0,0)" format="rgb" />

RGBA Mode

<field name="colour3" type="color" label="Colour" default="rgba(0,0,0,.5)" format="rgba" />

TO-DO

  • Add function to destroy the picker
    • Use the above function (once done) in subform fields
  • Add colour picker to new repo
    • Submit package to NPM
    • Use grunt to pull package via NPM

@dgt41 @ciar4n @wilsonge @Fedik

avatar C-Lodder C-Lodder - open - 31 May 2017
avatar Bakual
Bakual - comment - 31 May 2017

So this removes an external maintained JS (https://github.com/claviska/jquery-minicolors) in favor of an homebrewn one? Just because it requires JQuery?
Or do I miss something?

avatar C-Lodder C-Lodder - change - 31 May 2017
The description was changed
avatar C-Lodder C-Lodder - edited - 31 May 2017
avatar C-Lodder C-Lodder - change - 31 May 2017
The description was changed
avatar C-Lodder C-Lodder - edited - 31 May 2017
avatar joomla-cms-bot joomla-cms-bot - change - 31 May 2017
Category JavaScript Repository Layout Libraries
avatar C-Lodder
C-Lodder - comment - 31 May 2017

You're not missing anything @Bakual

avatar Bakual
Bakual - comment - 31 May 2017

To be honest, this sounds like NIH syndrom to me. Or a deep aversion against JQuery.
Which I both don't can't follow.

Imho, if you want to get rid of that JQuery colorpicker, you should look for an extisting solution based on the HTML5 color input (http://caniuse.com/#feat=input-color) and some polyfill for the IE11 and Safari (caniuse suggests https://github.com/jonstipe/color-polyfill but that requires JQuery as well g).

avatar C-Lodder
C-Lodder - comment - 31 May 2017
  • This is based on a script written by MDN. Not my own, just changes, and it will be added to a separate repo, so it can be maintained properly with the license that came with the original script.
  • Not going to justify why I went for vanilla over JQ
  • The HTML5 input colour has very limited styling.
  • The link to the repo you added is 6 years old, requires jQuery, jQuery UI and Modernizr....so no thanks
avatar C-Lodder C-Lodder - change - 31 May 2017
Labels Added: ?
avatar Bakual
Bakual - comment - 31 May 2017

The link to the repo you added is 6 years old, requires jQuery, jQuery UI and Modernizr....so no thanks

I didn't say to use that. It's a link from caniuse I just shared. There may be others.

This is based on a script written by MDN. Not my own, just changes, and it will be added to a separate repo, so it can be maintained properly with the license that came with the original script.

As soon as you change anything, it becomes our own fork and we need to maintain it. Doesn't matter if it's in a separate repo or not. You're then just hiding the fact that you hack a 3rd party ?

avatar brianteeman
brianteeman - comment - 31 May 2017

as @mbabker has repeatedly said we should not be hacking external libraries

avatar mbabker
mbabker - comment - 31 May 2017

We were discussing this last night and it sound like he had some reasoning to fork it and basically make it his own project. I'll let him explain his thinking a bit more, but at the first sound of it it wasn't just change for the sake of change.

avatar C-Lodder
C-Lodder - comment - 31 May 2017

This isn't a library that's on Github (that I know of), it's just a script that MDN published on Codepen and JSFiddle, therefore NOT maintained like other libraries are on Github. It had a hellish amount of rubbish, such as drag and drop, showing different shades of the colour selected, etc....which has now been removed.

Take a look for yourselves to see what I mean: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Colors/Color_picker_tool

avatar brianteeman
brianteeman - comment - 31 May 2017

i will stick by my comment - @mbabker has done a very good job in educating me that this is the correct way

avatar mbabker
mbabker - comment - 31 May 2017

I agree totally when it comes to libraries. In this particular case it's not really a library but rather a sample script one could use to build off of. Had they released it as a library we'd be having a different discussion, but for me this is one of those cases where it doesn't exactly fall into the "thou shalt not core hack your dependencies" preaching.

avatar Bakual
Bakual - comment - 31 May 2017

It still will be a library we have to maintain ourself (or @C-Lodder as long as he wants to do it).
The only "advantage" being that it is not depending on JQuery.

Doesn't look like it's worth to me.

avatar mbabker mbabker - change - 1 Jun 2017
Status New Pending
Build staging 4.0-dev
avatar C-Lodder
C-Lodder - comment - 2 Jun 2017

@dgt41 will this work for the destroy method? 3b43d7e

avatar brianteeman brianteeman - change - 8 Jun 2017
Milestone Added:
avatar brianteeman brianteeman - change - 8 Jun 2017
Milestone Added:
avatar C-Lodder
C-Lodder - comment - 24 Aug 2017

Closing as I'm going to make this a web component

avatar C-Lodder C-Lodder - change - 24 Aug 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-08-24 13:57:14
Closed_By C-Lodder
avatar C-Lodder C-Lodder - close - 24 Aug 2017

Add a Comment

Login with GitHub to post a comment