Form with controls and hooks #9
Conversation
Matushl
force-pushed
the
matushlavacik/form-with-controls-and-hooks
branch
from
b1039a3 to
51c6ddb
Compare
Matushl
force-pushed
the
matushlavacik/form-with-controls-and-hooks
branch
from
0f3376a to
1eab1a7
Compare
There was a problem hiding this comment.
I would omit i18n library from this package completely. This would force use to use it and also I am not really sure, how we would pass correct config into this (you want config as singleton, this would create completely new config without a way to customize it per-project.
I would just pass these as props and we will have to wrap it to custom component to provide translations
There was a problem hiding this comment.
thinking about it now... I would probably move all translations to root Form component and pass it down through context. In this case we only need to "wrap" the root component with initial settings.. Or maybe do a completely new context provider which provides these (defined globally like react query provider etc.)
There was a problem hiding this comment.
I've implemented FormTranslationsContext, it provides english translations as default and you can override them with wrapping your app with provider with value of your translations
| export type TextAreaControlProps = Omit< | ||
| ComponentProps<typeof FormControl>, | ||
| 'type' | 'render' | ||
| > & { rows: number }; |
There was a problem hiding this comment.
I think these types are not entirely correct, but we can fix this in later releases, since I need to test something out
packages/ui-informed/src/index.ts
Outdated
| /** | ||
| * FormWithControls | ||
| */ | ||
| export * as FormWithControls from './form'; |
There was a problem hiding this comment.
I am on the edge with this one, I am leaning towards not defining these aliases in this case and simply having the application use import * as Form from '@utima/ui-informed';.
There would be cases like Form.Form but you could also define custom alias, or use it without one. WHat do you think?
There was a problem hiding this comment.
I just don't like that if you let it only on the import in final application, then there will be things like Form.useDefaultSubmitActions. But I admit that it's maybe kinda unusual to create this named import inside of package, but on the other hand we are directly offering user of our package "the best way" how to use it and do not mess up with importing Checkbox from utima/ui insted of utima/ui-informed :-D
But I don't have strong opinion on that, I just stated my thoughts and leaving decision up to you (so far I'm not removing it)
- move hooks to form folder - replace `type + &` with `interface + extends` - improve package.json and more
Decisions made:
added i18n with hardcoded translations - there will be need for another task to make messages customizableConsciously left out (todos for the future PRs):
add possibility to customize translations(alredy in this PR)? create some general(columns was removed completely)FiledColas Form ui element