DataViews: Refactor the edit function to be based on discrete controls #64404
Conversation
youknowriad
added
[Type] Code Quality
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
One thing I noticed and that I would like to improve separately is that the "label" of the "datetime" control look very different than the other. I understand that we used a fieldset... but I believe that visually we should make it look the same. 
|
| } | ||
|
|
||
| export function getControlByType( type: string ) { | ||
| if ( Object.keys( FORM_CONTROLS ).includes( type ) ) { | ||
| return FORM_CONTROLS[ type ]; | ||
| } | ||
|
|
||
| return null; | ||
| throw 'Control ' + type + ' not found'; |
There was a problem hiding this comment.
Does this mean that, for TypeScript, getControlByType always returns a control? When implementing this same idea the issue I had was working with the null through all the chain of components.
There was a problem hiding this comment.
We we need to throw an error here?
There was a problem hiding this comment.
The reason, is what @oandregal said above, it's the way to force typescript to consider that this function should always return a defined control.
There was a problem hiding this comment.
I hadn't tested it but looks good.
I strongly prefer that dataform-controls live within src/components. As I mentioned here, the current folder layout doesn't help navigating and understanding the codebase. This is not a blocker, of course.
I understand, I wouldn't mind too much either but for me we have clear "concepts" or "abstractions": layouts, field types, controls... and these are not just components for me, they can have labels, sorting functions, icons... So putting them under |
#64404) Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Related #59745
Follow-up to #64370
What?
In #64370 The notion of "controls" has been introduced to the DataForm, this PR expands on that notion and transforms all the existing field types to rely on controls.
So a Field can say
Edit: "radio"but a Field Type can also sayEdit: "radio".One advantage as well is that we can remove the duplication of the "select" control between the different types and factor it in the
getControlfunction.Testing Instructions
There should be no impact on any public API or feature, this is a code quality change. You can check that the DataForm story is still the same.