[Widgets editor] Disallow multiple instances of reference widgets #26148
Conversation
adamziel added [Feature] Widgets Screen adamziel requested review from draganescu, tellthemachines and youknowriad as code owners
Size Change: +384 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
talldan force-pushed the update/prevent-multiple-reference-widgets branch from 22cdf5b to 76459f1 Compare
There was a problem hiding this comment.
I rebased, resolved merge conflicts, and push three fairly basic commits that are either bug fixes or code quality.
This looks good to me. There might be more we can do in the future like removing the 'duplicate' option for reference widgets, but I think we can push those things out to separate PRs as they're more debatable.
| ); | ||
| // Filter out used reference widgets | ||
| return filter( visible, ( widget, referenceWidgetName ) => { |
There was a problem hiding this comment.
Using filter here was converting the value from an object to an array, which caused an issue when mapping the options for the select control, as key is supposed to be the widget name instead of a number.
I've changed it to use the pickBy above in 76459f1.
However, I don't think an object should be used for SelectControl values, as it'll give unpredictable results in terms of ordering.
| icon={ <BlockIcon icon={ brush } /> } | ||
| label={ __( 'Duplicate Widget' ) } | ||
| > | ||
| <p className="components-placeholder__error"> |
There was a problem hiding this comment.
Hmm, this classname doesn't really seem to do much. I see the styles are defined in the placeholder styles, but I think it must be legacy given the lack of any notable styling. I'd expect some red text or something.
Ideally there would be a PlaceholderError component or an error prop on the placeholder that renders this. The pattern of using another component's classname like this is discouraged. Some more info:
https://github.com/WordPress/gutenberg/blob/master/docs/contributors/coding-guidelines.md#naming
I've switched this to use the instructions prop (in cb5df71), which makes it look pretty much the same, but without the code quality issues.
Description
This PR fixes #25494 by:
This PR only affects the frontend and does not change the actual API handlers.
How has this been tested?
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: