-
Notifications
You must be signed in to change notification settings - Fork 32
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
RFC: API redesign #125
Comments
Great idea! A random thought from my side would be to decouple any css framework compatibility from core. This has an advantage of others not having to bundle components/styles that aren't needed (similar to other addons which strip module from the addon tree). So something like the following configuration would allow a user to specify which component will be used for any validated-form form field type. This reduces copy pasting when specifying a custom rendering component. // config/environment.js
...
"ember-validated-form": {
"render": {
"text": "validated-input",
"textarea": "validated-textarea",
"label": "validated-label",
"checkbox": "validated-checkbox",
...
}
}
... One could maybe also allow aliasing so This could mean that the developer could still overwrite the renderComponent on a per form field basis but also get custom rendering for the whole project without having to copy paste it all over the place. I guess one could even combine it to allow default rendering for all form components but overwrite some: // config/environment.js
"ember-validated-form": {
"extends": "bootstrap",
"render": {
"label": "some-special-label"
...
}
} It's also possible to not have any magical extension logic but export some objects, which could be // config/environment.js
const {bootstrap} = require('ember-validated-form/configs');
"ember-validated-form": {
"render": Object.assign(bootstrap, {
"label": "some-special-label"
...
})
} |
Hi @makepanic Thank you for your feedback! 👍
Do you mean putting the themes in separate addons (e.g
You're absolutely right! We had the same idea but forgot to mention it in the RFC. I added it to the optional features, since this is not something crucial.
A global option for a default set of components is definitely reasonable. FYI: I already added a very very basic partial implementation of this RFC in #128. If you want, take a look at it and give us your feedback! |
I think both are valid solutions. Filtering the addon tree on build would be easier for you, i guess.
Thanks, I'll try to take a look and add feedback |
I think filtering it brings the advantage of not having to maintain x other addons for every existing framework while still decreasing the payload significantly.. Like this, we have all of the |
I think with lerna or yarn workspaces there wouldn't be much additional maintenance overhead while still having all With filtering you also have to maintain additional broccoli code that filters the addon tree. But it's up to you for what's easier to maintain. |
Well, we'd still have to maintain all of the dependencies for every addon even if we have it in one repo.. Or did I miss something? |
I'll go ahead and close this since the redesign has landed since a while 🙂 |
Many users of this addon had problem using it with their favorite CSS framework. The HTML output is currently too inflexible. To change this we propose the following new API for the
validated-input
component. Since this will break current integrations we'd like to get early feedback to this change.The new API could look something like this:
New features
errorComponent
to customize the errorshintComponent
to customize the hintsOptional features
renderComponent
to typesBreaking changes
config.css
will be dropped in favor of themingRelated issues
All feedback is very welcome!
CC: @czosel @bendemboski @jacob-financeit @makepanic @kimroen @omairvaiyani
The text was updated successfully, but these errors were encountered: