Skip to content

Introduce resolve props #651

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 4 commits into from
Jul 17, 2020
Merged

Conversation

rvsia
Copy link
Contributor

@rvsia rvsia commented Jul 15, 2020

closes #646

For more details read the documentation page

This PR

  • add resolveProps for form-connected fields
  • moves actionsMapper functionality to renderForm (so you can use actions for subForms etc.)
  • allows to merge mapper's actions and field's actions

Example of resolve props

{
    name: 'text_box_2',
    label: 'Validate me',
    isRequired: true,
    validate: [{
        type: 'required'
    }],
    component: components.TEXT_FIELD,
    resolveProps: (_props, {
        meta
    }) => {
        if (meta.valid && meta.touched) {
            return {
                helperText: 'Validated',
                validated: 'success'
            };
        }

        return {};
    }
}

poc

@Hyperkid123
Copy link
Member

We should follow the current order of priority Schema > GlobalProps. Merge the resolvers but if there are definitions in both component mapper and in the schema and share same keys, the schema key will override the mapper key.

{
  ...mapperResolver,
  ...schemaResolver
}

@rvsia rvsia force-pushed the resolveProps branch 3 times, most recently from ea97e23 to fbed84c Compare July 16, 2020 10:03
@codecov
Copy link

codecov bot commented Jul 16, 2020

Codecov Report

Merging #651 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
+ Coverage   93.00%   93.02%   +0.02%     
==========================================
  Files         189      189              
  Lines        3015     3025      +10     
  Branches      990      997       +7     
==========================================
+ Hits         2804     2814      +10     
  Misses        211      211              
Impacted Files Coverage Δ
...ges/react-form-renderer/src/files/use-field-api.js 90.90% <100.00%> (-0.52%) ⬇️
...act-form-renderer/src/form-renderer/render-form.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 146e2d3...8788317. Read the comment docs.

@rvsia rvsia requested a review from Hyperkid123 July 16, 2020 12:22
@rvsia rvsia added enhancement New feature or request renderer React form renderer PR labels Jul 16, 2020
@rvsia rvsia changed the title [WIP] Resolve props Introduce resolve props Jul 16, 2020
Copy link
Member

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

@rvsia can you open an issue to cache the use field return value? It should improve performance a bit.

@@ -9,6 +9,8 @@ export interface FieldActions {
[key: string]: FieldAction;
}

export type ResolvePropsFunction = (props: AnyObject, metaInput: AnyObject, formOptions: AnyObject) => AnyObject;
Copy link
Member

Choose a reason for hiding this comment

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

meta and form options types should be defined. You can use the final form meta props definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I missed formOptions, I will add it tomorrow.


export type FieldAction = [string, ...any[]];

export interface FieldActions {
[key: string]: FieldAction;
}

export interface InputMeta<FieldValue, T extends HTMLElement = HTMLElement> {
meta: FieldMetaState<FieldValue>;
input: FieldInputProps<FieldValue, T>;
Copy link
Member

Choose a reason for hiding this comment

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

I think you only want the meta typing right? Input is not passed to the function,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is.

Copy link
Member

Choose a reason for hiding this comment

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

Then the name is a bit confusing. I would expect just the meta object here. Can we change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's called InputMeta.. should it be InputAndMeta? :D

Copy link
Member

Choose a reason for hiding this comment

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

what about fieldApi?

@Hyperkid123 Hyperkid123 merged commit b56795d into data-driven-forms:master Jul 17, 2020
@Hyperkid123
Copy link
Member

🎉 This PR is included in version 2.8.0 🎉

The release is available on

Demo can be found here!

@rvsia rvsia deleted the resolveProps branch August 5, 2020 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released renderer React form renderer PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal - resolveProps
2 participants