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.

Sign up for GitHub

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

Jump to bottom

CustomPicker does not work in modern React #754

Open
hzhu opened this issue Aug 26, 2020 · 9 comments
Open

CustomPicker does not work in modern React #754

hzhu opened this issue Aug 26, 2020 · 9 comments

Comments

@hzhu
Copy link

hzhu commented Aug 26, 2020

Seems like there is a problem with CustomPicker higher-order component in React 16+. Can CustomPicker be used in React v16?

Works with React v15.6.1
https://codesandbox.io/s/custom-picker-old-react-forked-o9770

Does not work with React v16.12.0
https://codesandbox.io/s/custom-picker-new-react-ur2z4
@tazhel
Copy link

tazhel commented Aug 27, 2020

Same problem here, would love to have a hot-fix on this.

@hkurra
Copy link

hkurra commented Aug 28, 2020

Commenting getDerivedStateFromProps in ColorWrap.js has fixed the problem for me
image

but I am not fully aware of the root cause, there is one related thread, where it mentioned that there is a dev-dependencies on an older version of react

@avainola
Copy link

avainola commented Aug 28, 2020 •
edited
Loading

Root cause is #662 , componentWillReceiveProps can't be changed to getDerivedStateFromProps just like that.

React docs

Note that this method is fired on every render, regardless of the cause. This is in contrast to UNSAFE_componentWillReceiveProps, which only fires when the parent causes a re-render and not as a result of a local setState.

v2.17.3 works for me

@hkurra
Copy link

hkurra commented Aug 28, 2020 •
edited
Loading

@hzhu @tazhel if you listen onChange and provide the changed colour to colorpicker through color props then it is working fine, Here is the working codeSandBox Project forked from your one.
image

basically it is working in a controlled mode, not in AutoControlled mode or It is kind od providing your own CustomWrapper over color-picker instead of using ColorWrap

@xerigo
Copy link

xerigo commented Aug 28, 2020

Seems like there is a problem with CustomPicker higher-order component in React 16+. Can CustomPicker be used in React v16?

Works with React v15.6.1
https://codesandbox.io/s/custom-picker-old-react-forked-o9770

Does not work with React v16.12.0
https://codesandbox.io/s/custom-picker-new-react-ur2z4

hkurra pushed a commit to hkurra/react-color that referenced this issue Aug 30, 2020
- Made changes to run CustomPicker aka ColorWrap HoC in all version o…
b8bc27c 
…f react.

- Replaced getDerivedStateFromProps with componentDidUpdate and put a check on color input props before updating color state .

Fixed casesandberg#754
hkurra pushed a commit to hkurra/react-color that referenced this issue Aug 30, 2020
Merge branch 'master' of https://github.com/hkurra/react-color into fix/
48aa854 
casesandberg#754
@hkurra hkurra mentioned this issue Sep 11, 2020
Open
@rooraggio
Copy link

Any solutions on this one?

@monolithed
Copy link

monolithed commented Nov 1, 2020 •
edited
Loading

The same problem with ChromePicker and SketchPicker.

"react": "16.13.1",
"react-dom": "16.13.1",
"react-color": "2.19.3"

@yarindeohcld
Copy link

Any update on this thread ?

adamborowski added a commit to adamborowski/react-color that referenced this issue Dec 2, 2020
@adamborowski
fix for casesandberg#754 prevent ColorWrap to reset own state
c721662 
The issue is that ColorWrap HOC manages color state but at the same time accepts "color" prop which affects the state in getDerivedStateFromProps.
Because getDerivedStateFromProps is called every render, it always replaces state without acutal checking if the color property was changed.
In the demo https://codesandbox.io/s/custom-picker-new-react-ur2z4 mentioned in casesandberg#754 the component has initial value 'orange' - it will derive to state every render.

I added a step to check if property was changed to prevent resetting internal state.

But to be honest I suggest to change the API of the component to avoid confusion.
In the example
```tsx
<MyPicker color="orange" onChangeComplete={handleColorChange} />
```
Color prop is always passed so it should never change it by its own.
If the intention is to provide initial value for the component, I think we should add 'defaultValue' property to indicate that it will be used only once.
@adamborowski adamborowski mentioned this issue Dec 2, 2020
Closed
@adamborowski
Copy link

Fix provided in #791

@hzhu hzhu changed the title CustomPicker does not work in modern React CustomPicker (uncontrolled usage) does not work in modern React Dec 2, 2020
@hzhu hzhu changed the title CustomPicker (uncontrolled usage) does not work in modern React CustomPicker does not work in modern React Dec 9, 2020
@gffuma gffuma mentioned this issue Feb 23, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@monolithed @hzhu @adamborowski @hkurra @avainola @rooraggio @tazhel @xerigo @yarindeohcld