Skip to content
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

Better groups and new validation mechanic #484

Merged
merged 7 commits into from
Sep 24, 2018

Conversation

lionel-bijaoui
Copy link
Member

@lionel-bijaoui lionel-bijaoui commented Aug 27, 2018

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    New group system and revamped validation system.
  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    Change schema for groups
  • Other information:
    This is still a WIP, I tried to modify the group first but faced many issue related to the current validation system.
    So I went back and started to work on validation. Instead of using strongly bonded parent/child relationship, it now use an event bus. All event circulate on this event bus.
    As of now, it is more of a POC than the final implementation.
    I use the internal property "_uid" of vue objects and I don't think you can use more than one vue-form-generator by page (they will use the same event bus and no id is created, so validation one vfg will trigger all the validation) EDIT: fixed.
    I'm going to tackle these issues first, then, implement groups. At this point, validation should be independent from structure and allow more flexible solutions.
    @zoul0813 Please take a look if you have time and help me find unforeseen consequences.

@lionel-bijaoui
Copy link
Member Author

lionel-bijaoui commented Aug 27, 2018

Now instead of a global event bus for the whole component, a new one is created per instance and send down to children with props.
I have added a test with 2 instance of vfg on the same page and they are isolated from each other (in term of validation and events).

@lionel-bijaoui
Copy link
Member Author

Groups are working. There is a lot of changes.
form-group is now form-element and form-group is a recursive component that allow for the flexibility enabled by this new system.
form-element should be seen as a wrapper of fields. It take care of label, help, buttons, hint and error display, as well as the field in itself. It also take care of fieldID and classes.

A group is a special type of field.

{
    type: "group",
    legend: "My new group",
    styleClasses: "nice-group",
    tag: "section",
    fields: [...]
}

In a way, schema is just the main group of your form.

Now you can do that without problem (taken from the new grouping dev example)

schema: {
    fields: [
        {
            type: "group",
            legend: "Contact Details",
            tag: "div",
            fields: [
                {
                    type: "input",
                    model: "name",
                    label: "Name",
                    fieldOptions: {
                        inputType: "text"
                    },
                    required: true,
                    validator: ["required"]
                },
                {
                    type: "group",
                    legend: "Subgroup",
                    styleClasses: "subgroup",
                    tag: "fieldset",
                    fields: [
                        {
                            type: "input",
                            model: "subname",
                            label: "Name",
                            fieldOptions: {
                                inputType: "text"
                            },
                            required: true,
                            validator: ["required"]
                        }
                    ]
                },
                {
                    type: "input",
                    model: "email",
                    label: "Email",
                    fieldOptions: {
                        inputType: "email"
                    }
                }
            ]
        },
        {
            type: "input",
            model: "single",
            label: "Single field (without group)",
            fieldOptions: {
                inputType: "text"
            },
            required: true,
            validator: ["string"]
        },
        {
            type: "group",
            legend: "Other Details",
            fields: [
                {
                    type: "input",
                    model: "others.more",
                    label: "More",
                    fieldOptions: {
                        inputType: "text"
                    }
                },
                {
                    type: "input",
                    model: "others.things",
                    label: "Things",
                    fieldOptions: {
                        inputType: "text"
                    }
                }
            ]
        }
    ]
}

Many things have been changed, I tried to stop using Lodash forEach and use ES6 forEach instead. Nothing against Lodash, but I think the native implementation is good enough.
Other time, I used Lodash isNil, isFunction, etc instead of manually checking.

I also changed the way the fieldUID are created. Before, I was using the internal vm._uid but now it use a mix of fieldID and Lodash uniqueID. For now this uid is use very loosely, but I think that having them could be useful for future needs.

Now for the caveats and limitations I was able to detect.
Group don't validate. Validation is using an event bus and is uncorrelated to structure. Events don't traverse the structure tree so group don't know the validation state of their children.
There is possible solution to find this information and maybe uid are also opening other way to do that. I will not fix it right now (will be explored in future patch/minor version).

Manual validation return a promise and is asynchronous. Since everything communication through the event bus (and by event), there is no way to fall back toward synchronous validation for the whole form.
It doesn't change the way single validator works as far as I know.

<vue-form-generator ... ref="form"><vue-form-generator>
/* Old way */
myManualValidation() {
    let errors = this.$refs.form.validate();
    if(errors.length > 0) {
        // Validation error
        console.warn("Error during validation", error);
    } else {
        // No validation errors
        // ...

    }
}
/* New way */
myManualValidation() {
    this.$refs.form.validate().then(
        () => {
            // No validation errors
            // ...
        },
        (error) => {
            // Validation error
            console.warn("Error during validation", error);
        }
    );
}

@lionel-bijaoui
Copy link
Member Author

I'm going to work on the validation states now.
The idea is to implement a dirty variable to have a third validation state (untouched, error and valid).
I will have a lot of work toward tests and documentation. While working on that I noticed a lot of missing documentation or errors in it. This update will be jarring to many users since it will require them to change a lot of options. We cannot release something without an good documentation.
@vue-generators/vfg In the meantime, please challenge and test my PR. I certainly have done stupid things and your feedback is priceless. Thank you all !

@lionel-bijaoui lionel-bijaoui changed the base branch from master to v3 September 19, 2018 09:36
Update dep
@coveralls
Copy link

coveralls commented Sep 20, 2018

Coverage Status

Coverage decreased (-1.5%) to 87.5% when pulling c416802 on lionel-bijaoui:lb_better_groups into c0b65b5 on vue-generators:v3.

@lionel-bijaoui
Copy link
Member Author

I forgot to remove some big skip.
They are all about the new validation which is now a promise and not a synchronous validation.

- Remove some skip
- Fix some test
@lionel-bijaoui lionel-bijaoui merged commit 02055ec into vue-generators:v3 Sep 24, 2018
@lionel-bijaoui lionel-bijaoui deleted the lb_better_groups branch September 24, 2018 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants