Improve error handling when converting classic to block menus #4858
Conversation
|
Thank you for this PR and catching those Issues. Looks like the unit tests will need updating to conform to the changes. Do you think you will be able (have capacity) to do that? Also cc'ing @spacedmonkey as he may need to oversee this being merged into Core. |
|
This will also resolve WordPress/gutenberg#52481 |
|
Yep, it's on my list to update in the next week or so, I hope. |
|
@getdave Test updated! |
There was a problem hiding this comment.
The changes proposed in this PR fix discrepancies in how errors are handled in different situations.
LGTM (but please see #4858 (review)).
|
|
||
| if ( empty( $menu_items ) ) { | ||
| return array(); | ||
| return ''; |
There was a problem hiding this comment.
Is this a BC (backward compat) issue?
tl;dr I think no.
Why?
The method shipped in 6.3.0 is clearly documented as returning a string, not an array. Returning an array then is a bug that needs to be fixed.
Co-authored-by: Tonya Mork <tonya.mork@automattic.com>
hellofromtonya
left a comment
There was a problem hiding this comment.
LGTM 👍 Should be commit ready
|
Committed via https://core.trac.wordpress.org/changeset/56422. Thank you everyone for your contributions! |
Trac ticket: https://core.trac.wordpress.org/ticket/58823