Skip to content

fix(renderer): Do not override initial values with set values. #1225

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.

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

Merged
merged 1 commit into from
Apr 5, 2022

Conversation

Hyperkid123
Copy link
Member

Fixes #1224

Description

Field initial value is being overridden if a set condition is met during initialization phase

Schema (if applicable)

    const schema = {
      fields: [
        {
          component: 'text-field',
          name: 'field-1',
          label: 'first name',
        },
        {
          component: 'text-field',
          name: 'field-2',
          label: 'last name',
        },
        {
          component: 'text-field',
          name: 'field-3',
          label: 'occupation',
          condition: {
            sequence: [
              {
                and: [
                  { when: 'field-1', is: 'james' },
                  { when: 'field-2', is: 'bond' },
                ],
                then: { set: { 'field-3': 'SPY' } },
                else: { visible: true },
              },
              {
                and: [
                  { when: 'field-1', is: 'steve' },
                  { when: 'field-2', is: 'jobs' },
                ],
                then: { set: { 'field-3': 'CEO' } },
                else: { visible: true },
              },
            ],
          },
        },
      ],
    };

Checklist: (please see documentation page for more information)

  • Yarn build passes
  • Yarn lint passes
  • Yarn test passes
  • Test coverage for new code (if applicable)

@Hyperkid123 Hyperkid123 requested a review from rvsia April 5, 2022 07:31
@DataDrivenFormsBot
Copy link

A new version (fix) will be released: v3.16.11 [DataDrivenFormsBot]

@Hyperkid123 Hyperkid123 force-pushed the fix-sequence-initial-value branch from a87e7e5 to 3c7cbfa Compare April 5, 2022 07:32
@codecov
Copy link

codecov bot commented Apr 5, 2022

Codecov Report

Merging #1225 (ff17971) into master (1c46635) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1225   +/-   ##
=======================================
  Coverage   95.15%   95.15%           
=======================================
  Files         209      209           
  Lines        3591     3593    +2     
  Branches     1255     1256    +1     
=======================================
+ Hits         3417     3419    +2     
  Misses        174      174           
Impacted Files Coverage Δ
...ges/react-form-renderer/src/condition/condition.js 97.67% <100.00%> (+0.11%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

* If the field has an initial value and a conddition setter with positive result,
* do not override initial value with setter values.
*/
if (!meta.dirty && typeof meta.initial === 'undefined' ? true : meta.initial !== meta.value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be checking modified instead of dirty so after users change its value, the setter always run (even users go back to the initial value.)

meta.initial !== meta.value is dirty otherwise

Copy link
Member Author

@Hyperkid123 Hyperkid123 Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that would differ from current behavior. If a setter runs and then the user manually changes a field, the seter never overrides the manually changed value. This one is a real head scratcher.

@Hyperkid123 Hyperkid123 force-pushed the fix-sequence-initial-value branch from 3c7cbfa to ff17971 Compare April 5, 2022 08:54
@rvsia rvsia added bug Something isn't working renderer React form renderer PR labels Apr 5, 2022
@Hyperkid123 Hyperkid123 merged commit 857a56f into master Apr 5, 2022
@Hyperkid123 Hyperkid123 deleted the fix-sequence-initial-value branch April 5, 2022 09:09
@rvsia rvsia added the released label Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released renderer React form renderer PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using sequence in schema overwrites the initial values
3 participants