-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Page List Block: Fix critical error when converting to link #68076
Conversation
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, @t-hamano!
Size Change: +5 B (0%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
@Mamaduka Thanks for the review! |
Better yet, in this instance, we could not wrap this button in the Making the button always show by default and not ever have a value to be reset makes it completely pointless for it to appear in the tools panel menu. There's a quick and dirty hack (including a diff) over in this comment that explains more. |
Temporary fix for #67932 (comment)
What?
To prevent a critical error, add a
hasValue
prop to theToolsPanelItem
component that always returnsfalse
.Why?
In the case that this PR is trying to fix, only the "Edit" button is rendered, and it has no value. So, while ideally the
hasValue
prop itself wouldn't be necessary, this prop is required and will throw a critical error if it's missing.How?
Add a
hasValue
prop that always returnsfalse
to prevent a critical error. Ideally, I think theToolsPanelItem
should be modified to correctly handle items that have no value (See this comment).If this PR approach is not ideal, we may need to revert #67903 for now.
Testing Instructions
Screenshot