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 ofhandleFieldPath
(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 thehistory
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
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
betweenSmartForm
andForm
=>FormGenerator
would generate the whole form whileForm
would focus on form management. It would allow to have 3 behaviour:
- form + auto generation + graphql (this is currently the
SmartForm
- form + autogeneration without graphql (this is the
Form
currently) - 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.