plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
450 stars 611 forks source link

`formData` prop isn't kept in sync with internal `Form` state in `Form/Form.jsx` #5398

Open JeffersonBledsoe opened 10 months ago

JeffersonBledsoe commented 10 months ago

Describe the bug The Form component is allowed to have a formData prop passed to it which contains the blocks and block layout for the form to be rendered on the page. However, the current behaviour of this prop is that it is read in on component mount and copied to an internal state. The Form component then exclusively uses the internal state, ignoring any potential changes from the prop

To Reproduce Steps to reproduce the behavior:

  1. Render a Form component from @plone/volto/components/manage/Form/Form with a prop of formData
  2. Change the formData prop that is passed to it by the rendering component
  3. View the form on the web page

Expected behavior The form is updated to reflect the new formData

Actual behavior The form stays as it previously was

Options

davisagli commented 10 months ago

@JeffersonBledsoe It's hard to speculate about the right way to deal with conflicts without considering a specific use case. Is there a particular situation where you are anticipating updates to the prop while the component is active?

Sweetdevil144 commented 9 months ago

I've got a couple of ideas on how we can tackle this:

  1. Rename the Prop: We could rename formData to initialFormData. This change clarifies that the prop is only used for initial values and not expected to update.

  2. Sync Prop to State: We could also sync the formData prop to the state within componentDidUpdate, ensuring the internal state is always up-to-date with the props:

componentDidUpdate(prevProps) {
  if (this.props.formData !== prevProps.formData) {
    this.setState({ formData: this.props.formData });
  }
}

We need to decide on how to handle the form state once the user starts interacting with the form. Should the local state take precedence, or should updates to the formData prop override the user's input?

Would love to get your thoughts on this!

JeffersonBledsoe commented 9 months ago

Is there a particular situation where you are anticipating updates to the prop while the component is active?

I want to be able to render a full blocks form in a control panel. The Form component is the most direct way of achieving this currently. I recall having some issues when using the prop in combination with the onChangeFormData callback to keep the data in sync and be able to save the blocks data into the control panel, I'll get back to you

Sweetdevil144 commented 9 months ago

Let me have a look at the Form.jsx file. I'll remind you as soon. Also, if I remember correctly then GitHub also faced a similar issue with props in the past. I'll try to take some inspiration from those cases too. ^^

Sweetdevil144 commented 9 months ago

Thank you for elaborating on your objective to render a full blocks form in a control panel using the Form component. It's clear that keeping the form data in sync, especially when dealing with both the formData prop and the onChangeFormData callback, is crucial for your implementation.

Based on your description, it sounds like the synchronization between the external data (passed as formData prop) and the internal state of the form (which likely gets modified through user interaction and the onChangeFormData callback) is where the complexity lies.

A potential approach to address this could be to implement a mechanism within the componentDidUpdate method that checks for changes in the formData prop and intelligently merges these changes with the current state. This way, you ensure that the external updates are acknowledged while respecting the user’s interactions within the form.

Here's a rough sketch of what this could look like:

componentDidUpdate(prevProps) {
  if (this.props.formData !== prevProps.formData) {
    // Implement logic here to merge this.props.formData 
    // with the current state, potentially using a utility 
    // function that resolves conflicts between the two
    // while prioritizing user inputs where appropriate.
    this.setState({ formData: mergedFormData });
  }
}

As for handling conflicts between the prop and the internal state, a few strategies could be considered:

  1. Prioritizing User Input: If user modifications are critical, you might choose to prioritize the internal state changes made through user interactions over external formData prop updates.

  2. External Data as Source of Truth: Alternatively, if the control panel's data should always override user changes, the external formData updates can be given precedence.

  3. Hybrid Approach: In some cases, a hybrid approach that merges changes from both sources might be the most user-friendly. This would require a more sophisticated merging strategy to ensure that the form reflects a combination of both user changes and external updates without losing context.

I understand that you're still working through some issues with this approach. Please feel free to share any specific challenges you encounter, and I'd be happy to brainstorm solutions with you. Keeping the form data in sync, especially in dynamic and interactive interfaces like yours, often requires a bit of trial and error to get right.