From 243b8ccbb640c25221c9dc3246866453f6a6c3c7 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Fri, 19 Jan 2018 09:32:59 -0500 Subject: [PATCH 1/3] Add touch support for sliders include click-to-change on slider BG --- src/components/forms/slider.jsx | 25 +++++++++++++++++++++---- src/lib/touch-utils.js | 16 ++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 src/lib/touch-utils.js diff --git a/src/components/forms/slider.jsx b/src/components/forms/slider.jsx index f9e455e5..04fdbd87 100644 --- a/src/components/forms/slider.jsx +++ b/src/components/forms/slider.jsx @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import bindAll from 'lodash.bindall'; +import {getEventXY} from '../../lib/touch-utils'; import styles from './slider.css'; @@ -14,25 +15,39 @@ class SliderComponent extends React.Component { 'handleMouseDown', 'handleMouseUp', 'handleMouseMove', + 'handleClickBackground', 'setBackground' ]); } handleMouseDown () { - document.addEventListener('mouseup', this.handleMouseUp); document.addEventListener('mousemove', this.handleMouseMove); + document.addEventListener('mouseup', this.handleMouseUp); + document.addEventListener('touchmove', this.handleMouseMove); + document.addEventListener('touchend', this.handleMouseUp); } handleMouseUp () { - document.removeEventListener('mouseup', this.handleMouseUp); document.removeEventListener('mousemove', this.handleMouseMove); + document.removeEventListener('mouseup', this.handleMouseUp); + document.removeEventListener('touchmove', this.handleMouseMove); + document.removeEventListener('touchend', this.handleMouseUp); } handleMouseMove (event) { event.preventDefault(); + this.props.onChange(this.scaleMouseToSliderPosition(event)); + } + + handleClickBackground (event) { + this.props.onChange(this.scaleMouseToSliderPosition(event)); + } + + scaleMouseToSliderPosition (event){ + const {x} = getEventXY(event); const backgroundBBox = this.background.getBoundingClientRect(); - const x = event.clientX - backgroundBBox.left; - this.props.onChange(Math.max(0, Math.min(100, 100 * x / backgroundBBox.width))); + const scaledX = x - backgroundBBox.left; + return Math.max(0, Math.min(100, 100 * scaledX / backgroundBBox.width)); } setBackground (ref) { @@ -53,6 +68,7 @@ class SliderComponent extends React.Component { style={{ backgroundImage: this.props.background }} + onClick={this.handleClickBackground} >
); diff --git a/src/lib/touch-utils.js b/src/lib/touch-utils.js new file mode 100644 index 00000000..df1897b3 --- /dev/null +++ b/src/lib/touch-utils.js @@ -0,0 +1,16 @@ +/* DO NOT EDIT +@todo This file is copied from GUI and should be pulled out into a shared library. +See https://github.com/LLK/scratch-paint/issues/13 */ + +const getEventXY = e => { + if (e.touches && e.touches[0]) { + return {x: e.touches[0].clientX, y: e.touches[0].clientY}; + } else if (e.changedTouches && e.changedTouches[0]) { + return {x: e.changedTouches[0].clientX, y: e.changedTouches[0].clientY}; + } + return {x: e.clientX, y: e.clientY}; +}; + +export { + getEventXY +}; From 0279ba095c569702b7cba3d3346da06ad4366ef7 Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Fri, 19 Jan 2018 09:43:25 -0500 Subject: [PATCH 2/3] Enforce state update before propogating change through props. --- src/containers/color-picker.jsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/containers/color-picker.jsx b/src/containers/color-picker.jsx index 624e468c..cd06b03d 100644 --- a/src/containers/color-picker.jsx +++ b/src/containers/color-picker.jsx @@ -68,16 +68,19 @@ class ColorPicker extends React.Component { [50, 100, 100] : colorStringToHsv(color); } handleHueChange (hue) { - this.setState({hue: hue}); - this.handleColorChange(); + this.setState({hue: hue}, () => { + this.handleColorChange(); + }); } handleSaturationChange (saturation) { - this.setState({saturation: saturation}); - this.handleColorChange(); + this.setState({saturation: saturation}, () => { + this.handleColorChange(); + }); } handleBrightnessChange (brightness) { - this.setState({brightness: brightness}); - this.handleColorChange(); + this.setState({brightness: brightness}, () => { + this.handleColorChange(); + }); } handleColorChange () { this.props.onChangeColor(hsvToHex( From 7aff96c3ff01fd7524454bfe657ccc0c487fb1ca Mon Sep 17 00:00:00 2001 From: Paul Kaplan Date: Fri, 19 Jan 2018 09:58:47 -0500 Subject: [PATCH 3/3] Only update the full color state after eyedropper changes. This prevents the sliders from changing when other sliders change (i.e. bringing brightness down to 0). I needed to reorder the callback before deactivate so the color picker can know that the update is coming from the eyedropper. Also updated the comment to reflect what is really going on. --- src/containers/color-picker.jsx | 8 ++++---- src/containers/paint-editor.jsx | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/containers/color-picker.jsx b/src/containers/color-picker.jsx index cd06b03d..b36f38f6 100644 --- a/src/containers/color-picker.jsx +++ b/src/containers/color-picker.jsx @@ -28,9 +28,9 @@ const hsvToHex = (h, s, v) => parseColor(`hsv(${3.6 * h}, ${s}, ${v})`).hex ; -// Important! This component ignores new color props and cannot be updated -// This is to make the HSV <=> RGB conversion stable. Because of this, the -// component MUST be unmounted in order to change the props externally. +// Important! This component ignores new color props except when isEyeDropping +// This is to make the HSV <=> RGB conversion stable. The sliders manage their +// own changes until unmounted or color changes with props.isEyeDropping = true. class ColorPicker extends React.Component { constructor (props) { super(props); @@ -51,7 +51,7 @@ class ColorPicker extends React.Component { }; } componentWillReceiveProps (newProps) { - if (this.props.color !== newProps.color) { + if (this.props.isEyeDropping && this.props.color !== newProps.color) { // color set by eye dropper, so update slider states const hsv = this.getHsv(newProps.color); this.setState({ diff --git a/src/containers/paint-editor.jsx b/src/containers/paint-editor.jsx index 637e6a3d..ea2d5fe2 100644 --- a/src/containers/paint-editor.jsx +++ b/src/containers/paint-editor.jsx @@ -140,14 +140,14 @@ class PaintEditor extends React.Component { const callback = this.props.changeColorToEyeDropper; this.eyeDropper.remove(); - this.props.previousTool.activate(); - this.props.onDeactivateEyeDropper(); - this.stopEyeDroppingLoop(); if (!this.eyeDropper.hideLoupe) { // If not hide loupe, that means the click is inside the canvas, // so apply the new color callback(colorString); } + this.props.previousTool.activate(); + this.props.onDeactivateEyeDropper(); + this.stopEyeDroppingLoop(); this.setState({colorInfo: null}); } }