Re-render breaks form when serializedValue is set

[michielpauw]

Expected behavior

The issue relates to forms consisting of a radio group and/or list box. An option is pre-selected by setting serializedValue (for example, after a back navigation). When a different option is clicked, a callback gets called that in turn will result in an additional render.

Expected behaviour is that the option that was clicked is now selected and that modelValue has been updated accordingly.

Actual Behavior

modelValue is set to clicked value, but reset to serializedValue right away. This means that no other option can be selected than the preset serializedValue.

Additional context

Edit: some erroneous imports had slipped into the snippet.

See code below to reproduce the issue. The count property gets updated in the callback for @model-value-changed, resulting in a re-render. Original issue was raised because a team needed to call requestUpdate in the callback.

import { LocalizeMixin } from '@lion/localize';
import { LitElement, html, ScopedElementsMixin } from '@lion/core';
import { LionForm } from '@lion/form';
import { LionRadio, LionRadioGroup } from '@lion/radio-group';

export class TestForm extends LocalizeMixin(ScopedElementsMixin(LitElement)) {
  static scopedElements = {
    'lion-radio-group': LionRadioGroup,
    'lion-radio': LionRadio,
    'lion-form': LionForm,
  };

  static properties = {
    count: { type: Number },
  };

  constructor() {
    super();
    this.count = 0;
  }

  render() {
    return html` <lion-form
      name="dinosaurs"
      .serializedValue=${{ favoriteDinosaur: 'diplodocus' }}
    >
      <form>
        <lion-radio-group
          name="favoriteDinosaur"
          label="What is your favourite dinosaur ${this.count}?"
          @model-value-changed=${() => {
            this.count += 1;
          }}
        >
          <lion-radio
            label="Allosaurus"
            .choiceValue=${'allosaurus'}
          ></lion-radio>

          <lion-radio
            label="Brontosaurus"
            .choiceValue=${'brontosaurus'}
          ></lion-radio>

          <lion-radio
            label="Diplodocus"
            .choiceValue=${'diplodocus'}
          ></lion-radio>
        </lion-radio-group>
      </form>
    </lion-form>`;
  }
}

[Madses]

I've figured out a workaround until fixed. Seems to work without any issues for me. Given that it consistently defaults to ".serializedValue," we can ensure that this value stays current whenever a modification occurs. While not ideal, it serves the purpose for now.

 static properties = {
    count: { type: Number },
    currentValue : {type: String}
  };

  constructor() {
    super();
    this.count = 0;
    this.currentValue = 'diplodocus';
  }
  
render() {
    return html` <lion-form
      name="dinosaurs"
      .serializedValue=${{ favoriteDinosaur: this.currentValue }}
    >
      <form>
        <lion-radio-group
          name="favoriteDinosaur"
          label="What is your favourite dinosaur ${this.count}?"
          @model-value-changed=${(e) => {
            this.currentValue = e.target.modelValue;
            this.count += 1;
          }}
        >
          <lion-radio
            label="Allosaurus"
            .choiceValue=${'allosaurus'}
          ></lion-radio>

          <lion-radio
            label="Brontosaurus"
            .choiceValue=${'brontosaurus'}
          ></lion-radio>

          <lion-radio
            label="Diplodocus"
            .choiceValue=${'diplodocus'}
          ></lion-radio>
        </lion-radio-group>
      </form>
    </lion-form>`;

[Madses]

I believe I've found the cause of this issue. in ChoiceGroupMixin.js there is the following function.

      /**
       * @param {ChoiceInputHost} el
       * @param {string} val
       */
      const checkCondition = (el, val) => el.serializedValue.value === val;

      if (this.__isInitialSerializedValue) {
        this.registrationComplete.then(() => {
          this.__isInitialSerializedValue = false;
          this._setCheckedElements(value, checkCondition);
          this.requestUpdate('serializedValue');
        });
      } else {
        this._setCheckedElements(value, checkCondition);
        this.requestUpdate('serializedValue');
      }
    }

Here's whats happening:

1. If statement succesfully runs, and sets the checked value to value.
2. Right after this function runs again, but since this.__isInitialSerializedValue was set to false it then goes to the else statement. and sets the checked value once again, which is fine, but (i believe) unnecessary.
3. Every time we try to select a different radio option set serializedValue(value) gets invoked for some reason. The value briefly changes to the desired one, but then it immediately changes back to the initial serializedValue.

I am not sure what the desired result was of this specific part :

        this._setCheckedElements(value, checkCondition);
        this.requestUpdate('serializedValue');
      } 

So i'm not confident in proposing a solution yet. Although i did try a few things that seem to work and passed the tests.

One is simply removing this._setCheckedElements(value, checkCondition); in the else block. Another was instead of passing value to this._setCheckedElements inside the else block, we could instead pass this.modelValue as an argument. e.g. this._setCheckedElements(this.modelValue, checkCondition);

[gerjanvangeest]

The same behaviour can be found in other input fields. The serializedValue() is used to update the serializedValue every time the value changes, so it is part of our core value behaviour. Changing this is will be hard, since it will influence a lot of code.

What you could do is to set the serializedValue only once in firstUpdated(), and not on every paint.