-
Notifications
You must be signed in to change notification settings - Fork 468
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
fix(@clayui/color-picker): Update hue slider to use html range input #5156
Changes from all commits
c60938f
94752e9
7fcfd63
176b50b
20bc06c
7c8ee40
28ee9ae
7a50743
8332b5f
908ccf1
26b91e6
b3b1dbe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,9 @@ | |
* SPDX-License-Identifier: BSD-3-Clause | ||
*/ | ||
|
||
import React from 'react'; | ||
|
||
import {usePointerPosition} from './hooks'; | ||
import {hueToX, xToHue} from './util'; | ||
import {ClayInput} from '@clayui/form'; | ||
import ClaySlider from '@clayui/slider'; | ||
import React, {useState} from 'react'; | ||
|
||
type Props = { | ||
/** | ||
|
@@ -20,65 +19,79 @@ type Props = { | |
value: number; | ||
}; | ||
|
||
const useIsomorphicLayoutEffect = | ||
typeof window === 'undefined' ? React.useEffect : React.useLayoutEffect; | ||
|
||
/** | ||
* Renders Hue component | ||
*/ | ||
const ClayColorPickerHue = ({value = 0, onChange = () => {}}: Props) => { | ||
const containerRef = React.useRef<HTMLDivElement>(null); | ||
const selectorActive = React.useRef<boolean>(false); | ||
const [internalValue, setInternalValue] = useState<number>(value); | ||
|
||
const {onPointerMove, setXY, x, y} = usePointerPosition(containerRef); | ||
const handleOnChangeEnd = (event: any) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we won't have the internal state anymore, just add the |
||
onChange(internalValue); | ||
|
||
const removeListeners = () => { | ||
selectorActive.current = false; | ||
event.target.blur(); | ||
|
||
window.removeEventListener('pointermove', onPointerMove); | ||
window.removeEventListener('pointerup', removeListeners); | ||
event.target.focus(); | ||
}; | ||
|
||
useIsomorphicLayoutEffect(() => { | ||
if (containerRef.current && selectorActive.current) { | ||
onChange(xToHue(x, containerRef.current)); | ||
} | ||
}, [x]); | ||
|
||
React.useEffect(() => { | ||
if (containerRef.current) { | ||
setXY({x: hueToX(value, containerRef.current), y}); | ||
} | ||
setInternalValue(value); | ||
Comment on lines
36
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we are going to use the parent state There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't do this one due to #5156 (comment). |
||
}, [value]); | ||
|
||
React.useEffect(() => removeListeners, []); | ||
|
||
return ( | ||
<div | ||
className="clay-color-range clay-color-range-hue" | ||
onPointerDown={(event) => { | ||
event.preventDefault(); | ||
|
||
selectorActive.current = true; | ||
onPointerMove(event); | ||
|
||
(containerRef.current!.querySelector( | ||
'.clay-color-range-pointer' | ||
) as HTMLElement)!.focus(); | ||
|
||
window.addEventListener('pointermove', onPointerMove); | ||
window.addEventListener('pointerup', removeListeners); | ||
}} | ||
ref={containerRef} | ||
> | ||
<button | ||
className="clay-color-pointer clay-color-range-pointer" | ||
<div className="clay-color-form-group"> | ||
<ClaySlider | ||
className="clay-color-slider clay-color-slider-hue" | ||
max={360} | ||
min={0} | ||
onChange={setInternalValue} | ||
onKeyUp={(event) => { | ||
const arrowKeys = [ | ||
'ArrowDown', | ||
'ArrowLeft', | ||
'ArrowRight', | ||
'ArrowUp', | ||
]; | ||
|
||
if (arrowKeys.indexOf(event.key) > -1) { | ||
handleOnChangeEnd(event); | ||
} | ||
}} | ||
onPointerUp={handleOnChangeEnd} | ||
showTooltip={false} | ||
step={1} | ||
style={{ | ||
background: `hsl(${value}, 100%, 50%)`, | ||
left: x - 7, | ||
color: `hsl(${internalValue}, 100%, 50%)`, | ||
}} | ||
type="button" | ||
value={internalValue} | ||
/> | ||
<ClayInput.Group> | ||
<ClayInput.GroupItem> | ||
<ClayInput | ||
data-testid="hInput" | ||
insetBefore | ||
max="360" | ||
min="0" | ||
onChange={(event) => { | ||
const value = event.target.value; | ||
|
||
let newVal = Number(value); | ||
|
||
if (newVal < 0) { | ||
newVal = 0; | ||
} else if (newVal > 360) { | ||
newVal = 360; | ||
} | ||
|
||
onChange(newVal); | ||
}} | ||
type="number" | ||
value={value} | ||
/> | ||
<ClayInput.GroupInsetItem before tag="label"> | ||
H | ||
</ClayInput.GroupInsetItem> | ||
</ClayInput.GroupItem> | ||
</ClayInput.Group> | ||
</div> | ||
); | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need another state here, because we also have a state in the parent component, in which case we have access to them which are
value
andonChange
it is easier to maintain a source of truth than to have multiple states and keep synchronizing them all, this is also bad because of unnecessary rerenders.