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

Margin block supports: show by default in site title and site tagline #34060

Closed
wants to merge 1 commit into from

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Aug 13, 2021

Description

A PR to make margin controls display by default in the Dimensions ToolsPanel controls for site title and site tagline blocks.

Both these blocks already have margin supports. They are, at the time of writing, the only do that have.

Using the utterly tremendous __experimentalDefaultControls we can decide which controls we'd like to show, and which to leave collapsed.

Testing

Turn on some Lo-Fi beats.

Spin up this branch.

Insert site title and site tagline blocks, and make sure that the Dimensions control show in the sidebar and that the margin controls are visible by default.

margin-support-default

Types of changes

Turning on existing default panel features for two blocks.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

…so that they are displayed in the panel by default.
@ramonjd ramonjd added the [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi label Aug 13, 2021
@ramonjd ramonjd self-assigned this Aug 13, 2021
@ramonjd
Copy link
Member Author

ramonjd commented Aug 13, 2021

There was a question over in #34026 (comment) whether it'd be useful at all to have a default here.

An argument could be made that adding margins between site-significant blocks to space it from surrounding post content is useful. It's fairly tenuous I guess.

But another motivation for showing margin controls by default here was to avoid a collapsed Dimensions ToolPanel, and therefore confusion for the user.

It's a new feature – it cannot be "opened" by clicking on the toolbar header, like other panels – and its utility and function may not be obvious at first, particularly if it's collapsed.

collapsed-dimensions

Copy link
Contributor

@aaronrobertshaw aaronrobertshaw 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 putting this together @ramonjd 👍

✅ Site tagline margin control shows by default
✅ Site title margin control shows by default

This PR tests as advertised. I'd be inclined to approve this however I'm not sure as to whether we are really wanting to display the margin controls by default.

As you mention, simply displaying them by default for the sake of avoiding a visually empty panel is not the strongest of cases.

I was also of the understanding that we didn't want any margin controls to show by default.

The concerns around the use of the new panel were hopefully to have been reduced by moving the other block supports to use the ToolsPanel as well.

It would be good to get some other opinions on this. @jasmussen @apeatling @mtias Any thoughts?

@mtias
Copy link
Member

mtias commented Aug 16, 2021

This is too much of an early and local optimization. We still have a bit of a way to go in figuring out how to present padding and margin together. It's also likely that if a panel is completely empty, the ellipsis should become a + or something to that effect.

@ramonjd
Copy link
Member Author

ramonjd commented Aug 16, 2021

Thanks for the comments, and for testing.

I was also of the understanding that we didn't want any margin controls to show by default.

Good to have confirmed. This was a bit of a throw away PR, just to see how it appeared.

Seeing as the ToolsPanel is merged, we'll find out very quickly what folks' preferences are regardless 😄

It's also likely that if a panel is completely empty, the ellipsis should become a + or something to that effect.

This is a neat idea, and I think one that's worth evaluating. I'd like to try it out. I'll add it to my list.

🙇

@ramonjd
Copy link
Member Author

ramonjd commented Aug 16, 2021

Closing for now until the controls UI patterns, as they pertain to dimensions, evolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants