added control for the arrayRenderer, consolidate module, added styles #2011
Conversation
Signed-off-by: Ralph Soika <ralph.soika@imixs.com>
|
This is the new layout example:
|
|
Hi @sdirix I don't want to be impatient, but what did you think about my last pull request? |
|
Hi @rsoika, we'll take a look 👍 |
There was a problem hiding this comment.
Hi @rsoika ,
thank you very much for your pull request. In general, this already works great. I tested the changes with the vanilla dev example and did not run into any issues.
I added some comments regarding formatting and I think we should still export the ArrayControl to allow adopters to use it in their custom renderers and to not break the API in this regard.
|
|
||
| const { convertToValidClassName } = Helpers; | ||
|
|
||
| const ArrayControl = ({ |
There was a problem hiding this comment.
I think we should export the ArrayControl, too, in case adopters want to use it in custom renderers.
| }}>Up</button> | ||
| <button | ||
| className={buttonClassDown} | ||
| aria-label={`Down`} |
There was a problem hiding this comment.
Please align the indentation with the button's other props. Please also make sure to only uses spaces but not tabs
| if (window.confirm('Are you sure you wish to delete this item?')) { | ||
| removeItems(path,[index])(); | ||
| } |
There was a problem hiding this comment.
Please fix the indentation. The if should be indentated 2 spaces further than the onClick. E.g. see the onClick handler of the Up button.
| { | ||
| name: 'array.control', | ||
| classNames: ['array-control-layout', 'control'] | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Please remove the whitespace following the comma
| import { ArrayControl } from './ArrayControl'; | ||
|
|
||
| export { ArrayControl, ArrayControlRenderer }; | ||
| export { ArrayControlRenderer }; |
There was a problem hiding this comment.
Similarily to the comment in ArrayControlRenderer.tsx, I think we should export ArrayControl here.
You can get import it together with the ArrayControlRenderer like this:
import ArrayControlRenderer, { ArrayControl } from './ArrayControlRenderer';
and the change the export to
export { ArrayControlRenderer, ArrayControl };
| range(0, data.length).map(index => { | ||
| const childPath = composePaths(path, `${index}`); | ||
| return ( | ||
| <div key={index}><JsonFormsDispatch |
There was a problem hiding this comment.
Please add a new line before <JsonFormsDispatch to increase readability
Signed-off-by: Ralph Soika <ralph.soika@imixs.com>
|
Hi @lucas-koehler, I have fixed now the issues you mentioned. |
There was a problem hiding this comment.
@rsoika Thank you for the updates and thanks again for your contribution. LGTM now :)
also removed the file ArrayControl tsx and consolidated in ArrayControlRenderer.tsx