Skip to content

Fix: registerPlugin width issue #66725

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.

Sign up for GitHub

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

Jump to bottom
Closed
up1512001 wants to merge 2 commits into from
Closed

Fix: registerPlugin width issue #66725

up1512001 wants to merge 2 commits into from

Conversation

@up1512001
Copy link
Member

@up1512001 up1512001 commented Nov 4, 2024

What?

Fix the width issue for custom dropdown meta fields.

Why?

closes #66724

How?

added 100% width to components-panel__row child components-base-control class.

Testing Instructions

  1. add this code to gutenberg.php file
add_action( 'init', function() {
	register_post_meta( 'post', 'blog_date_info', array(
		'show_in_rest' => true,
		'single' => true,
		'type' => 'string',
		'default' => 'default',
	) );
} );

add_action( 'enqueue_block_editor_assets', function(){
	wp_enqueue_script( 'date-info', plugin_dir_url( __FILE__ ) . 'test.js', array( 'wp-plugins', 'wp-edit-post', 'wp-element', 'wp-components', 'wp-data' ) );
} );
  1. create test.js file in same directory of gutenberg.php file and add this build JS code
(()=>{var e={686:()=>{var e=wp.data,t=e.useSelect,a=e.useDispatch,r=wp.components.SelectControl,o=wp.editPost.PluginPostStatusInfo,n=wp.plugins.registerPlugin,__=wp.i18n.__,l=function(){var e=t((function(e){var t=e("core/editor").getEditedPostAttribute("meta");return{blogDateInfo:(null==t?void 0:t.blog_date_info)||"default"}})).blogDateInfo,o=a("core/editor").editPost;return React.createElement(r,{label:__("Display Date Format","rtcamp-features"),value:e,options:[{label:__("Default","rtcamp-features"),value:"default"},{label:__("Last updated on","rtcamp-features"),value:"updated"},{label:__("Published on","rtcamp-features"),value:"published"}],onChange:function(e){o({meta:{blog_date_info:e}})}})};n("blog-date-info-sidebar",{render:function(){return React.createElement(o,null,React.createElement(l,null))},icon:null})}},t={};function a(r){var o=t[r];if(void 0!==o)return o.exports;var n=t[r]={exports:{}};return e[r](n,n.exports,a),n.exports}a.n=e=>{var t=e&&e.__esModule?()=>e.default:()=>e;return a.d(t,{a:t}),t},a.d=(e,t)=>{for(var r in t)a.o(t,r)&&!a.o(e,r)&&Object.defineProperty(e,r,{enumerable:!0,get:t[r]})},a.o=(e,t)=>Object.prototype.hasOwnProperty.call(e,t),(()=>{"use strict";a(686)})()})();
  1. notice without width: 100% on components-base-control its cropping text and dropdown component is shrinked

Screenshots or screencast

Before After
Screenshot 2024-11-04 at 23 31 03 Screenshot 2024-11-04 at 23 37 40

@up1512001 up1512001 added the [Type] Bug An existing feature does not function as intended label Nov 4, 2024
@up1512001 up1512001 self-assigned this Nov 4, 2024
@up1512001 up1512001 requested a review from ajitbohra as a code owner November 4, 2024 18:14
@github-actions
Copy link

github-actions bot commented Nov 4, 2024 •
edited
Loading

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: up1512001 <up1512001@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@up1512001 up1512001 requested review from a team and t-hamano November 4, 2024 18:29
t-hamano
t-hamano reviewed Nov 5, 2024
View reviewed changes
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Since the PanelRow component is a Flex layout, it is correct in the specs that the child width is not 100%. Applying 100% width for a BaseControl-based component will affect all extenders that use the PanelRow component.

I think the underlying problem is that the PluginPostStatusInfo forces the child to be surrounded by the PanelRow component.

Until WP 6.5, the summary panel was a Panel component, so this may have made sense:

a1a51485eb9ef91f1cffb1e3862a31ec

But the current summary panel is not composed of the Panel component.

We might be able to consider removing the PanelRow wrapper in the PluginPostStatusInfo instead of making changes directly to the Panel component.

@t-hamano t-hamano requested a review from a team November 5, 2024 03:05
@Mamaduka
Copy link
Member

Mamaduka commented Nov 5, 2024

I think this also has been discussed in #41671, and the proposal was to improve documentation. See #60515.

tyxla
tyxla reviewed Nov 6, 2024
View reviewed changes
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@up1512001 thanks for the contribution! However, I'm not sure I agree with this change.

First, I don't see why the Panel component needs to declare any specific styles for BaseControl - they just have nothing to do with each other. A better place to have this would be in a stylesheet that's related to the place where this usage manifests - for example, in the post editor sidebar, or inspector controls.

Second, if there's no core need for this, I'm not sure it makes sense to have it in the first place. It can actually regress existing use cases that rely on the fact that the field doesn't currently take 100% of the width.

Last, I'd expect this is something the custom plugin handles, by introducing additional styles. After all, for some plugin UIs it might make sense to take 100% of the width, and for others it may not, so I'd just prefer the components to be less opinionated about it.

packages/components/src/panel/style.scss
Comment on lines +158 to +160
.components-base-control {
width: 100%;
}
Copy link
Member

@tyxla tyxla Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fair, I think this shouldn't live in the components package.

@t-hamano
Copy link
Contributor

t-hamano commented Nov 14, 2024

Thanks everyone for your feedback.

In any case, it seems like this issue shouldn't be solved in the components package, so let's close this PR.

Instead, we can move forward with #60515 to improve the documentation, or submit a PR for a different approach.

@t-hamano t-hamano closed this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tyxla tyxla tyxla left review comments

@t-hamano t-hamano t-hamano left review comments

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

Assignees

@up1512001 up1512001

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Custom sidebar pannel is not taking full width

4 participants

@up1512001 @Mamaduka @t-hamano @tyxla