Calling the `setPageData` action creator directly doesn't update values in the page data form

The current changes in the MR fix the problem, but only without React.StrictMode (can be removed in src/main.tsx). React.StrictMode renders every component twice, and that must be causing a race condition. This will only affect the app in development mode, but it's still a bug we need to fix.

My theory is that inputBehaviors initially sets the default value when the form element is rendered, but it's not reactive to changes that happen in pageDataSlice. This has gone unnoticed because previously changes were always carried out by users using the form, so the form would already reflect the most up-to-date values.

Yes, the value shown on the field is set internally in a useState, the values in the pageDataSlice don't go back the other way

#4: We're using a selector from pageDataSlice, so we're already subscribed to changes, but the internal state was not being updated. That's already fixed, now the suspected race condition remains (see #3).

Talked to @balintbrews about me having a crack at this since I've been working in this corner of XB quite often lately.

I'm leaning towards creating an additional action that is very similar to setPageData but specifically for use cases where the store change needs to inform the form vs the other (more common) way around.

We could use a custom event, we do something similar for when ajax updates the form values, albeit to update the page data slice, but input behaviours could generically listen for an event that tells it to update its internal value

I wonder why it is better to introduce a new API when we already have the pageDataSlice, to which inputBehaviors is already subscribed via a selector. My current MR simply adds logic to react to those changes and update the internal state.

I'm not attached to that approach, but aren't we painting over a bug <React.StrictMode> is showing us with the inconsistent re-renders?

Ah sorry, I missed that we took value from pageData before passing it to inputBehaviorsCommon - that seems like a clean approach

I could not get the original MR: 3533703-setpagedata-vs-form-values to work. I'm not sure what use cases it was able to work as the defaultValue (AFAIK) does not consult the pageData slice.

I tested that MR and my new one by installing the xb_test_article_fields module and adding a temporary button that calls setPageData

<button
        style={{ marginTop: '20px', border: '3px solid black' }}
        onClick={() => {
          dispatch(
            setPageData({
              ...formState,
              'body[0][value]': 'stringForTesting body field',
              'body[0][summary]': 'stringForTesting summary',
              'field_xbt_textarea[0][value]':
                '<p><b>stringForTesting bold</b></p>',
              'field_xbt_textarea_summary[0][value]':
                'stringForTesting textarea summary value',
              'field_xbt_textarea_summary[0][summary]':
                'stringForTesting textarea summary summary',
              field_xbt_options_buttons: 'option3',
              'path[0][langcode]': 'zxx',
              'field_xbt_textfield[0][value]':
                'stringForTesting textfield value',
              'field_xbt_unlimited_text[0][value]':
                'stringForTesting on the Coast',
            }),
          );
        }}
      >
        TEMPORARY BUTTON THAT UPDATES PAGE DATA
      </button>

In the 3533703-setpagedata-vs-form-values-refetch-on-update MR, this temporary button is also there but calls a newly added updatePageData action which is just setPageData but also triggers an event to inform the page data form it needs to refresh. I've confirmed this approach works with text fields, but not sure about radio/select etc yet.

It seems like there was some doubt about my approach so perhaps the ultimate solution is some combination of the two MRs?

the defaultValue (AFAIK) does not consult the pageData slice

My understanding is that it does, but I could easily be wrong. 🙂 Here is how I traced it:

1. In InputBehaviorsEntityForm, we subscribe to selectPageData;
2. pageData[fieldName] indirectly ends up as attributes.value or attributes.checked;
3. attributes is part of the props object that gets passed to InputBehaviorsCommon;
4. In InputBehaviorsCommon, getDefaultValue() reads from attributes that is previously destructured from props.

It seems like there was some doubt about my approach so perhaps the ultimate solution is some combination of the two MRs?

Not really a doubt, I was just wondering if the inconsistent rendering by React.StrictMode shows a potential bug. You and @larowlan know this area way more than I do, so I'm happy to accept your direction.

Found the issue we were overwriting the attribute.value in InputBehaviorsCommon, new MR up with a fix

The new MR 1253 looks promising - if we can avoid more build_id headaches I'm all for it.

I was unable to test manually until I made a change in the getPathAlias function DrupalPathWidget.tsx that checked for the truthiness of titleValue before it calls string-specific methods.

The need to do that might be related to what I then ran into:

While testing manually, I ran into an issue that seems to only occur if no fields in the page data form have been interacted with yet.
To reproduce:

• Reload the UI and immediately click the "temporary button that updates page data"
• Note that this updates the form field, but not the preview.
• Once any further interaction with the form takes place - regardless of field - the title field will reverts to the stale value shown in the preview.

.

Once any further interaction with the form takes place - regardless of field - the title field will reverts to the stale value shown in the preview.

That's because the button doesn't also write to the form state slice

Anything that uses this pattern will need to do both

That's because the button doesn't also write to the form state slice

Actually I think we could make inputBehaviors do this too

Once any further interaction with the form takes place - regardless of field - the title field will reverts to the stale value shown in the preview.

    // eslint-disable-next-line react-hooks/exhaustive-deps

was biting us here 😂

Pushed a simple fix in inputBehaviours

MR 1250 ready

One way to manually test is to add:

// PageDataForm.tsx
// update imports
import {
  selectPageData,
  setPageData,
  updatePageDataExternally,
} from '@/features/pageData/pageDataSlice';

// add a test button
/*
      <button
        style={{ marginTop: '20px', border: '3px solid black' }}
        onClick={() => {
          dispatch(
            updatePageDataExternally({
              'title[0][value]': 'stringForTesting title',
              'body[0][summary]': 'stringForTesting summary',
            }),
          );
        }}
      >
        TEMPORARY BUTTON THAT UPDATES PAGE DATA
      </button>
 */

I'm assuming tests for the modules that wind up using this will

I'm assuming tests for the modules that wind up using this will

The code in the AI module is already merged. If we think this needs test coverage and if we're not adding tests here, we make sure to open a follow-up to ensure those tests get written. Unless there's some integration code needed for this.

Automatically closed - issue fixed for 2 weeks with no activity.