Hide prefilled props on SmartForm when fields restraint is specified

[Neobii]

When a fields restraint and prefilled props are specified on SmartForm, it will add the fields to the values to the form but hide them if they are not inside the fields array, that way, prefilled props will be submitted and you won't need to specify hidden on the schema.

[SachaG]

Can you clarify what the current behavior is, and what the desired behavior should be?

[Neobii]

How it is now, you have to specify hidden:true on the schema in order for it to not show up in the SmartForm fields and it still submit.

But what I'm thinking is, when defining a SmartForm you could do this <SmartForm fields={["name"]} prefilledProps={{gender: "male"}}/> and have the gender submit in the form while being hidden without it having gender be hidden in the schema of the field. That way you could show/hide it on a form by form basis.

[SachaG]

Right so another way to put it is you'd like prefilledProps not to be restricted to the fields that are present in the form?

[Neobii]

Yup, however then, the name prefilledProps might not be completely intuitive.

[SachaG]

prefilledValues would be better I think? I'm not even sure why I used the word props in the first place. I think this would require a little tweaking of the way prefilledProps/values are used currently, but it should not be that hard to add.

@eric-burel can you see any downsides to doing this?

[eric-burel]

Seems a reasonable behaviour to me, I'd expect the fields props to be always respected when displaying, and prefilledProps too when submitting. I agree with the prefilledValues name too.

What I don't get yet is why we get this behaviour currently:

• PrefilledProps are merged into the initial document. That's the only place they are used directly. https://github.com/VulcanJS/Vulcan/blob/devel/packages/vulcan-forms/lib/components/Form.jsx#L94
• Then, the fields (fields = visible inputs in the form) are computed based on the document and user rights. https://github.com/VulcanJS/Vulcan/blob/devel/packages/vulcan-forms/lib/modules/schema_utils.js https://github.com/VulcanJS/Vulcan/blob/devel/packages/vulcan-forms/lib/components/Form.jsx#L314

I don't get why the prefilledProps are not respected when the field is not in the fields array. I guess it gets removed during form submission. Edit: I get it now:

• Issue seems to be here. Should be smth like const fields = [...this.getFieldNames(), ...Object.keys(this.props.prefilledProps)] instead, right now only the fields of the document that matches visible inputs get sent, non visible prefilledProps are ignored https://github.com/VulcanJS/Vulcan/blob/devel/packages/vulcan-forms/lib/components/Form.jsx#L226
• getFieldNames could be renamed getVisibleFieldNames to be clearer. We could write a more global getDocumentFieldNames that would include visible fields + prefilledProps fields. @Neobii Are you in for writing a small PR for this (and of course, unit tests :D) ?

Expected behaviour is simply to decorrelate fields and prefilledProps props: it should display only the provided fields (respecting user's authorizations), and it should always add the prefilledProps to the document, even when they don't show up in the form.

[Neobii]

I can write a pr for this :). Is there any difference between specifying hidden on the root object vs specifying it in the input properties?

[Neobii]

Also

inputProperties: {
      addonBefore: <Icon>attach_money</Icon>,
      defaultValue: () => 2424,
      disabled: true
    }

doesn't seem to submit 2424 as a value and

  startDate: {
    type: Date,
    label: "Start Date",
    optional: true,
    canCreate: ['members'],
    canRead: ['guests'],
    autoValue: function() {
      return moment(d).add(1, "M").toDate();
    }
  },

doesn't seem to add the value to submit, do you plan on supporting autoValue, I know you want to get away from simple schema eventually but it's so dang helpful :).

[Neobii]

The autoValue runs when I take off the canCreate and canRead from the schema, but when I add it to the mutation fragment that is returned from the SmartForm, it says

Error: GraphQL error: Cannot query field "startDate" on type "Event".
GraphQL error: Cannot query field "endDate" on type "Event".
    at new ApolloError (modules.js?hash=c5fad458235c229ee5485493ea45c7b1c0c0e5f7:85720)
    at modules.js?hash=c5fad458235c229ee5485493ea45c7b1c0c0e5f7:86634
    at meteor.js?hash=33066830ab46d87e2b249d2780805545e40ce9ba:1223

however if I take it off the fragment, everything runs just fine but it doesn't run the autoValue part.

[SachaG]

Those seem like separate issue but

1. defaultValue is a property of the field, not the inputProperties property.
2. instead of autoValue we have onCreate.

[Neobii]

Sorry for deviating a little bit! (Default value as input properties)[https://github.com/VulcanJS/vulcan-docs/blob/master/source/schema-properties.md#inputproperties]. What I was getting at here is that hidden should probably be apart of inputProperties as it describes the way the input is rendered. Perhaps, when I'm working on this I can shuffle some things to inputProperties and write deprecation notices to solidify the SmartForm api more.

[SachaG]

What I was getting at here is that hidden should probably be apart of inputProperties as it describes the way the input is rendered.

I think that could be misleading as it might imply that it'll be rendered as a hidden input, whereas it's actually not rendered at all.

Also inputProperties (if I'm not mistaken) is specifically for properties that you want to pass to the input, and of course if the input is not rendered it doesn't make sense to pass it properties.

By the way, Autoform had a form sub-property which would take properties targeting the field when displayed in a form but I found that confusing as I could never remember which properties were part of the form sub-property, and which properties were part of the general field schema. Which is why I think it's easier if everything is at the top level.

But maybe the name hidden itself is confusing then?