feat: add RadioGroup middleware #918
Conversation
|
This pull request has been deployed to:
|
| middleware: { icache: icacheFactory() }, | ||
| properties: () => ({}), | ||
| children: () => [] | ||
| })(onValueStub); |
There was a problem hiding this comment.
I am not 100% sure, but I think the build is failing because this is expecting the initialValue argument
|
|
||
| return (key: string) => ({ | ||
| checked(checked?: boolean) { | ||
| const existingValue = icache.get('value'); |
There was a problem hiding this comment.
I think this should be typed as a string icache.get<string>('value'); ?
|
|
||
| export const radioGroup = factory(({ middleware: { icache } }) => { | ||
| return (onValue: (value: string) => void, initialValue: string) => { | ||
| const existingInitialValue = icache.get('initial'); |
There was a problem hiding this comment.
I think this should be typed as a string: icache.get<string>('initial');?
There was a problem hiding this comment.
it's typed above @JamesLMilner via RadioGroupICache
| const factory = create({ radioGroup, theme }).properties<RadioGroupProperties>(); | ||
|
|
||
| export const RadioGroup = factory(function({ properties, middleware: { radioGroup, theme } }) { | ||
| const { name, label, options, renderer, onValue, initialValue } = properties(); |
There was a problem hiding this comment.
you could do initialValue = '' here rather than use || below.
There was a problem hiding this comment.
Looks good to me @bitpshr . Just needs the conflicts sorted and the test fixed. @JamesLMilner there's no need for a screenshot as there's a link automatically added to the PR to the example runner.
|
This pull request has been deployed to:
|
|
This pull request has been deployed to: |
|
This pull request has been deployed to:
|
Type: feature
The following has been addressed in the PR:
theme.composelike thisDescription:
This pull request adds a new
RadioGroupwidget and associated middleware.Resolves #798