Skip to content
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

Update: Make block inspector self contain its styles #17880

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Oct 10, 2019

This PR makes the block inspector component self contain its styles. Previously part of its styles were in the edit post module, which made the block inspector not display correctly on the widgets screen and on any third party use case.
With this change, we also remove the number of styles needed.

As it is possible to see this change does not touches the widget screen but the inspector there now shows as expected, which means this change benefits any case where the block inspector was shown outside of the edit post.

How has this been tested?

Verified the block inspector looks as before in normal post edit screen.
Verified the block inspector shows as expected on the widgets screen.

Screenshots

Widgets screen before:
Screenshot 2019-10-10 at 10 08 16
Widgets screen after
Screenshot 2019-10-10 at 09 52 09

@jorgefilipecosta jorgefilipecosta added [Package] Block editor /packages/block-editor [Feature] Widgets Screen The block-based screen that replaced widgets.php. [Type] Enhancement A suggestion for improvement. labels Oct 10, 2019
@jorgefilipecosta jorgefilipecosta added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Oct 10, 2019
@@ -1,3 +1,17 @@
.block-editor-block-inspector {
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see how this simplifies things but does it still hold true if we decide to show the inspector in a Modal tomorrow instead of a sidebar panel? Will we need these styles as well in the modal?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes if the inspector was in a modal these would still improve the situation.
What these styles do is the following:

  • Make sure that when we have a series of PanelBody components the borders does not appear twice (now that I think about it it would probably be a good idea to put this at the component level, I will try to see if there is a style that makes sense there).
  • Increase the spacing in fields inside base control e.g: so the fields are not very close to each other and the inspector has more space.

Both actions would also be good in a modal.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, both these styles also seem to be something that can be absorbed by the UI components themselves.

@jorgefilipecosta jorgefilipecosta force-pushed the update/make-the-block-inspector-generic-remove-unrequired-styles branch from 2c95fdb to 6966b34 Compare October 10, 2019 09:23
@jorgefilipecosta jorgefilipecosta force-pushed the update/make-the-block-inspector-generic-remove-unrequired-styles branch 3 times, most recently from 5a186ee to 92b7979 Compare October 10, 2019 15:36
@jorgefilipecosta jorgefilipecosta force-pushed the update/make-the-block-inspector-generic-remove-unrequired-styles branch from 92b7979 to abf85da Compare October 10, 2019 17:18
@jorgefilipecosta jorgefilipecosta merged commit 780860f into master Oct 10, 2019
@jorgefilipecosta jorgefilipecosta deleted the update/make-the-block-inspector-generic-remove-unrequired-styles branch October 10, 2019 20:51
@youknowriad youknowriad added this to the Gutenberg 6.7 milestone Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widgets Screen The block-based screen that replaced widgets.php. [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants