VulcanJS/vulcan-npm

SmartForm + Components registering using Context

eric-burel opened this issue · 2 comments

To do

Step 1: get the form to work with Vulcan API

  • Document a bit by creating a readme
  • Check if there is a way to automatically reload data in the update form => useUpdate do not seem to be run again when the data has been updated elsewhere
  • Check if textarea/enter keypress is ok, see packages/react-ui/components/form/FormComponentInner.tsx
  • Currently we need to explicitely pass currentUser to the SmartForm => it should be provided in the context? The User model belongs to Vulcan Next so we need to carefully check for this dependency.
  • In packages/react-components/components/form/Form/fields.ts, move the side effect of handleFieldPath (default value setting) somewhere else, to make the function pure
  • Allow to have the same form twice. For this, we need to make sure that input's id are unique, like "context + name"' (where it's user responsibility to give a unique "context" to their form). Note that id is mandatory for inputs so we can label them respecting a11y.
  • Pass callbacks through context, provides hooks etc. => ok for core components of the form. To be done for inputs, not sure of the best strategy, because that means needing a huge refactor. We could provide those values both through props and let the user also use the context?
  • Provide doc on how to create your own components
  • We need to figure out fragment registering. Some parts of the Form are relying on "expandQueryFragments" from "packages/vulcan-lib/lib/modules/fragments.js". What to do with those?
  • Get "Locales" from context in packages/react-components/components/form/FormIntl.tsx
  • Correct props for the context that provides the components
  • Correct input props, whitelist correctly when using an HTML input => started the work, but we still to correctly differentiate "HTML INPUT" and "HTML SELECT" correctly
  • Figure out "beforeComponent" / "afterComponent", and "instantiateComponent" pattern
  • Reintroduce existing unit tests => we could use this as an occasion to use the new storybook testing plugin, to load the form from stories directly? So that we don't duplicate decorators and all
  • Implement the SmartForm, that queries actual graphql (=FormContainer.tsx)
  • Fix Storybook Testing React misconfiguration storybookjs/testing-react#27
  • Fix issue with circular dependency: in packages/react-ui/components/form/tests/Form.stories.tsx, when importing ./Form, we have a circular dependency with packages/react-ui/components/form/defaultVulcanComponents.ts, not sure how to fix it... Currently we hard copy the Form folder to Form2 to duplicate the components but that's not very good.... => solution is to split the context provider with default component from the context consumer with the hooks, this avoids having an explicit circular dependency
  • Figure how to set the context => Story ok, nested dependencies seems to work ok
  • How to handle depencies between components that belongs the same context? => OK
  • Handle getLabel context, eg in packages/react-components/components/form/FormError.tsx
  • Switch to TS
  • Switch to stateless => done
  • Finish the FormLeavingManager component to handle the warnOnUnsavecChanges feature (we need to figure what is the history object exactly + stop triggering the message on page leave. We might need to recode the history.block feature?
    vercel/next.js#12348
    https://github.com/ReactTraining/history/blob/master/docs/blocking-transitions.md
    vercel/next.js#5377
    vercel/next.js#2476 (comment)
    vercel/next.js#2694
    => ok for browser events. See #40 for SPA specific routing, this belongs to Vulcan Next

image

Step 2: improve the API: better naming, clearer components structure etc.

  • Move graphql logic of the success callbacks out of Form, and put it into FormContainer
  • Introduce an intermediate FormGenerator between SmartForm and Form => FormGenerator would generate the whole form while Form would focus on form management. It would allow to have 3 behaviour:
  1. form + auto generation + graphql (this is currently the SmartForm
  2. form + autogeneration without graphql (this is the Form currently)
  3. just the form and custom inputs (this is not possible yet)

Others

About custom components

It is already possible to use a custom component as input:

{
   type: String,
   input: MySuperReactInput
}

Limitation is that you cannot override this component using the context, it's hard written in your schema.

We could allow something like this:

{ 
   type: String,
   input: "MyCustomInput"
}

The advantage of this pattern is that you can change "MyCustomInput" from the context, as other Vulcan inputs. It can be interesting if you want to implement an API that allows some component swapping or programming a multitenant app.
You basically can create your own Vulcan app.

Limitation is that this pattern, which already exists in Vulcan Meteor, tends to be deadly over-used: since the reference to the component is implicit, you may unknowingly create circular dependency, reference a component that doesn't exist etc.
So I am not in a hurry to add this feature. This might need another ticket.

Currently there's an issue about the fields you can get with the SmartForm: the only fields that appear are the ones which are usable (in creation or update depending on the way you use it) by "guests" , even if you're connected.

It seems like the SmartForm never gets the userId and so never gives it to the Form while rendering. The Form assumes you're not connected and renders only the guests-authorized fields.

Another issue about the Form and not the SmartForm but they are closely linked:
When you give to the Form an userId, it doesn't render the "admins" fields.

I think that getInsertableFields and getEditableFields functions, used by the Form and located in schema_utils, don't know how to recognize an admin yet.