Radio Group Renderer for Vue Vanilla (WIP) #2292
Conversation
✅ Deploy Preview for jsonforms-examples ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Hi @davewwww , thanks for the contribution ❤️
Sorry for the late review, we must have overlooked this PR 😅
I think the RadioGroupControl.vue could definitely be a cell. However, I would not do this in this PR but rather contribute this PR first and then potentially refactor it in the cell PR.
Could you add some unit tests for the renderers, please? We aim to have at least basic coverage for all renderers.
Regarding the css classes. Congruent with #2239 (comment), I think the new radio group properties should be added in a new child object of control. E.g.:
export const defaultStyles: Styles = {
control: {
root: 'control',
wrapper: 'wrapper',
label: 'label',
description: 'description',
input: 'input',
radioGroup: {
group: 'radio-group',
item: 'radio-group-item',
itemLabel: 'radio-group-item-label',
}
// [...]
}
}|
|
||
| export const entry: JsonFormsRendererRegistryEntry = { | ||
| renderer: controlRenderer, | ||
| tester: rankWith(20, and(isEnumControl, optionIs('format', 'radio'))), |
There was a problem hiding this comment.
Minor: rank 20 seems unnecessarily high considering standard renderers usually use values between 1 and 5. E.g. the plain enum renderer uses 2.
| tester: rankWith(20, and(isEnumControl, optionIs('format', 'radio'))), | |
| tester: rankWith(5, and(isEnumControl, optionIs('format', 'radio'))), |
|
|
||
| export const entry: JsonFormsRendererRegistryEntry = { | ||
| renderer: controlRenderer, | ||
| tester: rankWith(20, and(isOneOfEnumControl, optionIs('format', 'radio'))), |
There was a problem hiding this comment.
Minor: rank 20 seems unnecessarily high considering standard renderers usually use values between 1 and 5. E.g. the plain enum renderer uses 2.
| tester: rankWith(20, and(isOneOfEnumControl, optionIs('format', 'radio'))), | |
| tester: rankWith(5, and(isOneOfEnumControl, optionIs('format', 'radio'))), |
Hi,
this is a proposal for the radio group renderer, both enum and oneOf, when format=radio.
As I am also working on the Cell Renderer Set I wondered whether this renderer is actually more of a cell instead of renderer
Furthermore, the question of
Stylesnaming, as is currently being discussed here: Support different classes for different controlsIs
radioGroupa child of control or a new type at root level?What do you think?
Preview: https://deploy-preview-2292--jsonforms-examples.netlify.app/vue-vanilla/#radio-group