Fix internalUrl Widget to Reflect Prop Changes via onChangeBlock
When you update a block's properties using onChangeBlock, the widget may not show these changes because its state isn't updating correctly.
https://github.com/plone/volto/assets/50819975/8243fe43-2531-49c4-8405-49f9d998666b
The expected behavior is that the URL field in the sidebar will update with the new URL value, as it was updated by onChangeBlock.
π Documentation preview π: https://volto--6036.org.readthedocs.build/
Deploy Preview for plone-components ready!
| Name | Link |
|---|---|
| Latest commit | a230cb87f25052ca4f1f79649db49073c06623bb |
| Latest deploy log | https://app.netlify.com/sites/plone-components/deploys/66695ecf6d8a9e00088432b5 |
| Deploy Preview | https://deploy-preview-6036--plone-components.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
I've set up a test using coresandbox to demonstrate the bug I'm referring to.
Sorry for the mistake, I have put the right name back.
@davisagli I've modified the logic to take the value from the prop as you've suggested. It was made this way because this widget is basically a copy of the URLWidget made by redturtle to have internal links which was done by Edw as part of the content rules and is only used on the content rules. Still some cleanup todo but the issue described here would affect also the original URL widget.
One change that I am pushing for is to avoid having undefined value when we clear because React complains about it about controlled vs non controlled inputs. Having an empty string as value keeps the return and input of the url as a string and it keeps the state in sync. have a look at this video to see how it behaves when we use undefined onClear vs an empty string.
This video shows the behavior in action https://github.com/plone/volto/assets/152852/29dacd78-3d49-4b87-9a6c-8dcdcb710bd6
@ichim-david I think that's probably okay since an empty string evaluates to boolean False in Javascript -- as long as the backend accepts it when configuring a content rule. The other option would be setting it to null
@ichim-david I think that's probably okay since an empty string evaluates to boolean False in Javascript -- as long as the backend accepts it when configuring a content rule. The other option would be setting it to
null
@davisagli you can't You cannot pass value={undefined} first and later pass value="some string" because React wonβt know whether you want the component to be uncontrolled or controlled. A controlled component should always receive a string value, not null or undefined. https://react.dev/reference/react-dom/components/input#im-getting-an-error-a-component-is-changing-an-uncontrolled-input-to-be-controlled
We get warnings and it doesn't work well if we have null or undefined. This is why I propose the empty string since it evaluates to false and it keeps the value in sync without the need for the use effect