feat(checkbox): allow configuration of default checkbox options #139
Conversation
|
So we discussed it and decided to go with this idea. A few comments on general approach:
|
There was a problem hiding this comment.
Please update the PR according to my comment, keeping constant and token in the same file so it's easy to see default values. We can practice the approach on this PR and checkbox component and once we all happy with how it works we can go over the rest of the components.
Let me know what you think. I also added a section to the docs. Not sure if you want to have the different members of the type described in more detail (e.g. in a table or something). |
|
@waterplea Any update on this one. Would be really cool to have a template that we can use for the other components as well. |
| }; | ||
|
|
||
| /** Injection token that can be used to specify checkbox options. */ | ||
| export const TUI_CHECKBOX_OPTIONS = new InjectionToken<CheckboxOptions>( |
There was a problem hiding this comment.
@fynnfeldpausch @MarsiBarsi so what do you think if we will use this same token for icons config? If we will, then we need to move that to PrimitiveCheckbox. Sounds like a reasonable thing to do, I wouldn't want to create too many tokens.
There was a problem hiding this comment.
Good question. Don't know how to achieve a good separation then, as the checkbox might define other inputs as the primitive checkbox. One could either use different tokens for every component type (checkbox, checkbox-block, primitive checkbox,...) or go for a unified one with all fields and use it in every component. Don't know where to place it then. More interestingly, how would one make a distinction between the default size of the primitive checkbox and the normal checkbox component. Or use one size option and use it in every component (so basically not making them configurable individually)?...
There was a problem hiding this comment.
I was thinking one token per component. In this particular case, looks like Checkbox will just inject the same token.
| } | ||
|
|
||
| /** Default values for the checkbox options. */ | ||
| export const TUI_CHECKBOX_DEFAULT_OPTIONS: CheckboxOptions = { |
There was a problem hiding this comment.
This one is also Readonly<CheckboxOptions> so nobody decides to mutate the constant instead of providing :) We love readonly here :)
There was a problem hiding this comment.
Other than that one little comment this looks good to me. @MarsiBarsi ?
* main: chore(demo): copy spaces styles on click (taiga-family#203) feat(kit): allow configuration of default checkbox options (taiga-family#139) chore(i18n, demo): update i18n instructions (taiga-family#208) fix(i18n): correct inconsistencies in English translations (taiga-family#207) feat(i18n): add Dutch with 100% support (taiga-family#206) chore(i18n): add Turkish to exports feat(i18n): add Turkish with 100% support (taiga-family#200) chore(demo): fix import sample # Conflicts: # projects/demo/src/modules/markup/typography/typography.template.html
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Partially implements #133
What is the new behavior?
Default checkbox options can be set via a provider. This is just an example to have a discussion base for #133
Does this PR introduce a breaking change?