Move components in wp.blocks.InspectorControls.* to wp.components.*. #4817
Conversation
jorgefilipecosta
added
the
[Status] In Progress
blocks/inspector-controls/index.js
Outdated
| class WrappedComponent extends Component { | ||
| componentDidMount() { | ||
| // eslint-disable-next-line no-console | ||
| console.warn( `wp.blocks.InspectorControls.${ componentName } is deprecated use wp.components.${ componentName }` ); |
There was a problem hiding this comment.
Maybe wp.blocks.InspectorControls.${ componentName } is deprecated, use wp.components.${ componentName }.
Added punctuation.
blocks/inspector-controls/index.js
Outdated
| }; | ||
|
|
||
| forEach( | ||
| { BaseControl, |
There was a problem hiding this comment.
Should we move BaseControl to a new line aswell?
jorgefilipecosta
force-pushed
the
update/moved-ic-inputs-to-wordpress-components
branch
from
9196c84 to
db81465
Compare
jorgefilipecosta
removed
the
[Status] In Progress
|
Thank you for your review @youknowriad, this PR was rebased and the feedback was addressed. |
|
While I agree with this change, I added some reviewers to have other opinions here. |
There was a problem hiding this comment.
Not sure we need the nesting of input-controls within components.
I don't recall the specific reason why it was implemented the way it was originally; if we thought that the inputs were specific to usage within blocks/editor? As noted, they seem generic enough, so I don't have much of a problem with this.
| import { __ } from '@wordpress/i18n'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import BaseControl from './../base-control'; | ||
| import Button from '../../button'; |
There was a problem hiding this comment.
It's a shame this doesn't work, but it's frustrated me in the past in my attempts to do so:
import { Dashicon, withInstanceId, Button } from '../../';There was a problem hiding this comment.
Yes, this syntax would be better :(
jorgefilipecosta
force-pushed
the
update/moved-ic-inputs-to-wordpress-components
branch
4 times, most recently
from
2a7ad80 to
ddebc1c
Compare
|
Maybe at the time, these controls were created the idea was for them to be InspectorControls specific maybe even render them automatically inside inspector controls, but that ended up not being the case and the components ended up being generic. I rebased this PR adding some required changes meanwhile. All the feedback was addressed namely, removed the input-controls nesting and added a readme.md file for each component documenting them. |
The components were not inspector controls specific, they were just generic input elements. This move will allow other parts of the Gutenberg besides blocks, e.g: document sidebar to take advantage of this reusable components.
jorgefilipecosta
force-pushed
the
update/moved-ic-inputs-to-wordpress-components
branch
from
ddebc1c to
8f0c832
Compare
jorgefilipecosta
deleted the
update/moved-ic-inputs-to-wordpress-components
branch
|
@jorgefilipecosta @aduth seems like the Custom CSS Class is now missing. I've run build package and install the zip file. I'm not seeing the field now. Thanks! |
|
Thank you for reporting this problem @phpbits. I'm also able to replicate it, and I'm investigating its cause. |
|
@jorgefilipecosta you're very welcome. Let me know how it goes. I'm also using Inspector Control on my plugin integration and also not showing. Hope you can share the fixes. Thanks! |
|
@jorgefilipecosta I'll wait for the pull request to be merged and will try it on my end. I hope this will fix my issues too. Thanks! |
|
@jorgefilipecosta Yes fields are now showing but it's not saving. Have you tried adding values to custom CSS Class field and save the changes? Thanks! |
The components were not inspector controls specific, they were just generic input elements. This move will allow other parts of the Gutenberg besides blocks, e.g: document sidebar to take advantage of this reusable components.
This PR comes after some discussions in #4175 (review).
This move will also allow the move of TermTreeSelect, as TreeSelect component. Making possible to use it in the HierarchicalTermSelector removing existing duplicate code.
The inputs in Document sidebar could also be updated to use the components e.g: toggles (Stick to the Front Page) and selects (Author, Template) could make use ToggleControl and SelectControl, instead of repeating the same logic.
We expose a select control so Fixes: #3931;
Deprecation warnings are shown when importing the components from wp.blocks.InspectorControls.*, but for now, the import continues to work as before.
How Has This Been Tested?
This refactor affects the whole application so besides executing automatic tests, do general smoke testing around the app and verify things are working as before. Give special relevance to inspector controls.