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

Add touch events + corrections (IE8-...) + class prefix isolation + improvements #47

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

aruben-c
Copy link

@aruben-c aruben-c commented Jun 28, 2016

Hi,
I made some corrections and improvements to your ColorPicker.
You can see all of them in my GitHub repository.

Corrections:

  • IE8- fail with 'new' in same instance
  • mouseup event not fire on IE8-
  • document can differ from window.document
  • class prefix to isolate from higher class weight

Improvements:

  • Add touch events "touchstart", "touchmove" and "touchend" to also support touch devices.
  • class prefix to isolate from higher class weight
  • namespace
  • invalid hidden characters
  • Better addStyleSheet function

Tests:
Touch events successfully tested on a smartphone with Android 4.4.2 / Chrome 51.
IE8- tests OK
All others tested in IE8/11/Edge, Chrome, Firefox, Opera.

3 files are changed:

  • colorPicker.js
  • colorPicker.data.js
  • colors.js

I also impacted into:

  • index.html, color.all.min.js
  • javascript_implementation\index.html , jsColor.js , jsColorPicker.min.js
  • jQuery_implementation\index.html , jqColor.js , jQueryColorPicker.min.js

but, it's need to also change:

  • colorPicker.js.map
  • package.json (for the new version)
  • perhaps also the web site page, README.md, info.txt (because touch events are now added and others improvements).

For touch events, the files "javascript_implementation/jsColor.js" and "jQuery_implementation/jqColor.js" don't need to be changed (despite they call the event "mousedown").
But I changed them for the namespace.

The namespace add a constraint: the variable "var CPNamespace = window;" (or "var CPNamespace;") needs to be defined before loading the files.
(nb: the first parameter "window" of the functions can't be the namespace due to the use of "window.Math").

Thanks for this nice color picking widget.
ARuben

aruben-c added 12 commits June 28, 2016 19:34
Add the events "touchstart", "touchmove" and "touchend" to also support touch devices.
Need to change the files "README.md", "color.all.min.js", "javascript_implementation/jsColorPicker.min.js"
Successfully tested on a smartphone with Android 4.4.2 / Chrome 51
IE8- fail with 'new' in same instance
Simplification of the function "addStyleSheet()" using innerHTML instead of createTextNode
@aruben-c aruben-c changed the title Add touch events Add touch events + corrections (IE8-...) + class prefix isolation + improvements Aug 18, 2016
Add a namespace to allow storing the colorpicker into "namespace" instead "window"
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Need: create the variable CPNamespace before calling executing the script.
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Need: create the variable CPNamespace before calling executing the script.
For IE8- : use attachEvent/detachEvent instead of addEventListener/removeEventListener
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Need: create the variable CPNamespace before calling executing the script.
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Need: create the variable CPNamespace before calling executing the script.
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Need: create the variable CPNamespace before calling executing the script.
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Need: create the variable CPNamespace before calling executing the script.
Add a namespace to allow storing the colorpicker into "namespace" instead "window".
Need: create the variable CPNamespace before calling executing the script.
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.

1 participant