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

Allow negative values for margin inputs #40464

Merged
merged 4 commits into from
Apr 27, 2022

Conversation

JustinyAhin
Copy link
Member

What?

Allow negative values for margin inputs. Fixes #32644

Why?

See #32644 for details.

How?

The PR adds a new spacingType parameter to the BoxControl() function, and allows negative values for the input control in case this parameter is set to "Margin"

Testing Instructions

  • Add to a post a group block
  • Try to set the block margin to a negative value
  • The input should accept the negative value and the margin should apply to the block

@github-actions
Copy link

github-actions bot commented Apr 20, 2022

Size Change: +67 B (0%)

Total Size: 1.23 MB

Filename Size Change
build/block-editor/index.min.js 151 kB +23 B (0%)
build/components/index.min.js 222 kB +28 B (0%)
build/edit-site/index.min.js 47.5 kB +16 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 993 B
build/annotations/index.min.js 2.77 kB
build/api-fetch/index.min.js 2.27 kB
build/autop/index.min.js 2.15 kB
build/blob/index.min.js 487 B
build/block-directory/index.min.js 6.51 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-editor/style-rtl.css 15.1 kB
build/block-editor/style.css 15.1 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 111 B
build/block-library/blocks/audio/style.css 111 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 59 B
build/block-library/blocks/avatar/style.css 59 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 445 B
build/block-library/blocks/button/editor.css 445 B
build/block-library/blocks/button/style-rtl.css 560 B
build/block-library/blocks/button/style.css 560 B
build/block-library/blocks/buttons/editor-rtl.css 292 B
build/block-library/blocks/buttons/editor.css 292 B
build/block-library/blocks/buttons/style-rtl.css 275 B
build/block-library/blocks/buttons/style.css 275 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 103 B
build/block-library/blocks/code/style.css 103 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-template/style-rtl.css 127 B
build/block-library/blocks/comment-template/style.css 127 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-query-loop/editor-rtl.css 95 B
build/block-library/blocks/comments-query-loop/editor.css 95 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/cover/editor-rtl.css 546 B
build/block-library/blocks/cover/editor.css 547 B
build/block-library/blocks/cover/style-rtl.css 1.53 kB
build/block-library/blocks/cover/style.css 1.53 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 417 B
build/block-library/blocks/embed/style.css 417 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 353 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 961 B
build/block-library/blocks/gallery/editor.css 964 B
build/block-library/blocks/gallery/style-rtl.css 1.51 kB
build/block-library/blocks/gallery/style.css 1.51 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 333 B
build/block-library/blocks/group/editor.css 333 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/editor-rtl.css 731 B
build/block-library/blocks/image/editor.css 730 B
build/block-library/blocks/image/style-rtl.css 529 B
build/block-library/blocks/image/style.css 535 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 199 B
build/block-library/blocks/latest-posts/editor.css 198 B
build/block-library/blocks/latest-posts/style-rtl.css 447 B
build/block-library/blocks/latest-posts/style.css 446 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 493 B
build/block-library/blocks/media-text/style.css 490 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 708 B
build/block-library/blocks/navigation-link/editor.css 706 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation-submenu/view.min.js 375 B
build/block-library/blocks/navigation/editor-rtl.css 2.03 kB
build/block-library/blocks/navigation/editor.css 2.04 kB
build/block-library/blocks/navigation/style-rtl.css 1.93 kB
build/block-library/blocks/navigation/style.css 1.92 kB
build/block-library/blocks/navigation/view-modal.min.js 2.65 kB
build/block-library/blocks/navigation/view.min.js 395 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 363 B
build/block-library/blocks/page-list/editor.css 363 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 260 B
build/block-library/blocks/paragraph/style.css 260 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 69 B
build/block-library/blocks/post-comments-form/editor.css 69 B
build/block-library/blocks/post-comments-form/style-rtl.css 521 B
build/block-library/blocks/post-comments-form/style.css 521 B
build/block-library/blocks/post-comments/editor-rtl.css 77 B
build/block-library/blocks/post-comments/editor.css 77 B
build/block-library/blocks/post-comments/style-rtl.css 511 B
build/block-library/blocks/post-comments/style.css 511 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 721 B
build/block-library/blocks/post-featured-image/editor.css 721 B
build/block-library/blocks/post-featured-image/style-rtl.css 153 B
build/block-library/blocks/post-featured-image/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 323 B
build/block-library/blocks/post-template/style.css 323 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 80 B
build/block-library/blocks/post-title/style.css 80 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 370 B
build/block-library/blocks/pullquote/style.css 370 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 234 B
build/block-library/blocks/query-pagination/style.css 231 B
build/block-library/blocks/query/editor-rtl.css 369 B
build/block-library/blocks/query/editor.css 369 B
build/block-library/blocks/quote/style-rtl.css 213 B
build/block-library/blocks/quote/style.css 213 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 397 B
build/block-library/blocks/search/style.css 398 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 140 B
build/block-library/blocks/separator/editor.css 140 B
build/block-library/blocks/separator/style-rtl.css 233 B
build/block-library/blocks/separator/style.css 233 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 759 B
build/block-library/blocks/site-logo/editor.css 759 B
build/block-library/blocks/site-logo/style-rtl.css 181 B
build/block-library/blocks/site-logo/style.css 181 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 177 B
build/block-library/blocks/social-link/editor.css 177 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.37 kB
build/block-library/blocks/social-links/style.css 1.36 kB
build/block-library/blocks/spacer/editor-rtl.css 332 B
build/block-library/blocks/spacer/editor.css 332 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 504 B
build/block-library/blocks/table/editor.css 504 B
build/block-library/blocks/table/style-rtl.css 625 B
build/block-library/blocks/table/style.css 625 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/tag-cloud/style-rtl.css 226 B
build/block-library/blocks/tag-cloud/style.css 227 B
build/block-library/blocks/template-part/editor-rtl.css 149 B
build/block-library/blocks/template-part/editor.css 149 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 571 B
build/block-library/blocks/video/editor.css 572 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/common-rtl.css 993 B
build/block-library/common.css 990 B
build/block-library/editor-rtl.css 10.3 kB
build/block-library/editor.css 10.3 kB
build/block-library/index.min.js 177 kB
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/style-rtl.css 11.5 kB
build/block-library/style.css 11.5 kB
build/block-library/theme-rtl.css 689 B
build/block-library/theme.css 694 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/blocks/index.min.js 47 kB
build/components/style-rtl.css 15 kB
build/components/style.css 15 kB
build/compose/index.min.js 11.2 kB
build/core-data/index.min.js 14.5 kB
build/customize-widgets/index.min.js 11 kB
build/customize-widgets/style-rtl.css 1.39 kB
build/customize-widgets/style.css 1.39 kB
build/data-controls/index.min.js 663 B
build/data/index.min.js 8.65 kB
build/date/index.min.js 32 kB
build/deprecated/index.min.js 518 B
build/dom-ready/index.min.js 336 B
build/dom/index.min.js 4.58 kB
build/edit-navigation/index.min.js 15.8 kB
build/edit-navigation/style-rtl.css 4.05 kB
build/edit-navigation/style.css 4.05 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/edit-post/index.min.js 30.1 kB
build/edit-post/style-rtl.css 7.11 kB
build/edit-post/style.css 7.11 kB
build/edit-site/style-rtl.css 7.95 kB
build/edit-site/style.css 7.93 kB
build/edit-widgets/index.min.js 16.3 kB
build/edit-widgets/style-rtl.css 4.41 kB
build/edit-widgets/style.css 4.4 kB
build/editor/index.min.js 38.5 kB
build/editor/style-rtl.css 3.71 kB
build/editor/style.css 3.71 kB
build/element/index.min.js 4.29 kB
build/escape-html/index.min.js 548 B
build/format-library/index.min.js 6.62 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.66 kB
build/html-entities/index.min.js 454 B
build/i18n/index.min.js 3.79 kB
build/is-shallow-equal/index.min.js 535 B
build/keyboard-shortcuts/index.min.js 1.83 kB
build/keycodes/index.min.js 1.41 kB
build/list-reusable-blocks/index.min.js 1.75 kB
build/list-reusable-blocks/style-rtl.css 838 B
build/list-reusable-blocks/style.css 838 B
build/media-utils/index.min.js 2.94 kB
build/notices/index.min.js 957 B
build/nux/index.min.js 2.12 kB
build/nux/style-rtl.css 751 B
build/nux/style.css 749 B
build/plugins/index.min.js 1.98 kB
build/preferences/index.min.js 1.2 kB
build/primitives/index.min.js 949 B
build/priority-queue/index.min.js 628 B
build/react-i18n/index.min.js 704 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.69 kB
build/reusable-blocks/index.min.js 2.24 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 11.2 kB
build/server-side-render/index.min.js 1.61 kB
build/shortcode/index.min.js 1.52 kB
build/token-list/index.min.js 668 B
build/url/index.min.js 1.99 kB
build/vendors/react-dom.min.js 38.5 kB
build/vendors/react.min.js 4.34 kB
build/viewport/index.min.js 1.08 kB
build/warning/index.min.js 280 B
build/widgets/index.min.js 7.21 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.07 kB

compressed-size-action

packages/components/src/box-control/index.js Outdated Show resolved Hide resolved
packages/components/src/box-control/index.js Outdated Show resolved Hide resolved
Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

Thank you for the modifications @JustinyAhin!

This looks good to me, and seems to work as expected 👍

@JustinyAhin JustinyAhin merged commit a109d31 into trunk Apr 27, 2022
@JustinyAhin JustinyAhin deleted the add/negative-margin-to-controls branch April 27, 2022 06:27
@github-actions github-actions bot added this to the Gutenberg 13.2 milestone Apr 27, 2022
} ) {
inputProps = allowNegativeValues ? { min: -Infinity } : defaultInputProps;
Copy link
Contributor

Choose a reason for hiding this comment

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

@JustinyAhin Won't this overwrite inputProps, making the prop unusable?

Copy link
Member Author

Choose a reason for hiding this comment

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

@talldan in which scenario would that happen? Not sure I really understand your question

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm you have a point @talldan! Apologies, I initially missed this in my initial review.

Should probably be something like this instead:

inputProps.min = allowNegativeValues ? -Infinity : defaultInputProps.min;

What do you think?

@ndiego
Copy link
Member

ndiego commented Apr 27, 2022

As much as I want negative margin support. I do think we need to consider how a block with a negative margin will interact with those around it in the Editor. It may become extremely difficult to edit, and there are also z-index issues.

@aaronrobertshaw
Copy link
Contributor

Thank you for tackling negative margins @JustinyAhin 👍

Ultimately, I'd love to see negative margins supported and the designs they can unlock realised.

At this stage though, I think we might have merged these changes a little prematurely. I agree with @ndiego that allowing negative margins as they are, introduces more potential problems than they solve.

There are also a few other minor issues I think needed to have been addressed before merging this PR.

With limited testing and explorations so far some of the issues I see include:

  • Interactions within the editor are clunky once blocks begin to overlap. e.g. z-index and block selection problems.
  • Any change to the components within wordpress/components should have a changelog entry
  • Any new props should be documented in the readme (unless __experimental)
  • As raised elsewhere we could have avoided the new prop entirely which also avoids having to support it in the future
  • Allowing negative values within the BoxControl required further updates to how "mixed" values are determined so a single negative margin value is still displayed via the BoxControl's linked state.
  • Could the margin block support's visualizer have been extended in a way to reflect negative values (I'm not saying it's easily achievable but rather something we might look at).

Some of the above are obviously trivial and can still be done after the fact. The editor problems though need further exploration. Are we happy with leaving the potentially broken UX in the editor in order to keep negative margins?

I'd like to stress supporting negative margins would be great and I personally appreciate the effort put into this but I don't think we should be regressing the editor's UX to achieve it without due consideration.

Screen.Recording.2022-04-28.at.3.20.40.pm.mp4

@aristath
Copy link
Member

I opened #40674 to revert this PR if that's what we want to do 👍

@seothemes
Copy link

I kindly disagree that this would introduce more problems than it solves. Currently theme and plugin authors have to implement their own solutions because there is no standardized way. This creates more problems in the long run.

@aaronrobertshaw in the screen recording, isn’t that the expected behavior? If I was to enter a negative value, I would expect it to work exactly like that.

There’s many worse things that users can do to break the page layout very easily. For example align full and align wide. Or the navigation block. It can take users hours to reorganise their block structure, whereas fixing the above issue is a simple matter of removing the negative value from the input.

Another thing worth noting is that sometimes negative margins just don’t work. This is not a block editor issue, but CSS in general. At least allowing negative values would work most of the time.

Is there anything that can be done to help solve this problem? I know that it’s a deal breaker for many designers.

@aaronrobertshaw
Copy link
Contributor

Thanks @seothemes for taking the time to share your thoughts 👍

In the screen recording, isn’t that the expected behavior? If I was to enter a negative value, I would expect it to work exactly like that.

The recording also demonstrates that single negative margin values aren't displayed as such in the margin box-control which is not expected behaviour.

Whether the overall behaviour shown in terms of block selection was expected or not might be a separate question from whether that behaviour is desirable at the moment.

Some initial questions that come to mind around it might be:

  • How will negative margins impact or be constrained by layout block supports and styles?
  • How many users are aware of, and frequently use, the list view for block navigation vs keyboard vs mouse selection etc?
  • What UX impacts are there for users on smaller screens or mobile?
  • Should negative margins be something that an average user control or would limiting it to themes limit the potential conflicts as those negative values would be applied in a deliberate manner?
  • Can we have something like the parent block button where if there are multiple blocks under the cursor they are shown in the block toolbar for easy selection?
  • If blocks are to overlap, should we also be offering control over z-index as well?
  • What effects would custom z-index values have on other editor features like toolbars, padding/margin visualizers etc?
  • Could we limit negative margins to a sane value that prevents a lot of the block navigation/UX issues?

I'm sure there'll be a lot of other aspects to consider as we explore how best to add support for negative margins.

There’s many worse things that users can do to break the page layout very easily

Personally, this gives me further pause towards adding another way to break layouts.

Another thing worth noting is that sometimes negative margins just don’t work. This is not a block editor issue, but CSS in general. At least allowing negative values would work most of the time.

This is a good point.

My understanding of the general approach in Gutenberg is to try and avoid situations where users are setting values and nothing changes. There are already issues with this in regards to left/right margins depending on the context a block finds itself in. I don't know whether that makes it better or worse with regards to adding negative margins.

Is there anything that can be done to help solve this problem? I know that it’s a deal breaker for many designers.

There's definitely a demand for negative margin support. I expressed my desire to see it in my earlier comment also.

In terms of moving this issue forwards, I think we need to address concerns around how negative margins affect interactions in the editor.

Perhaps our best bet is a threaded discussion, to outline what the UX issues are, gather ideas to mitigate them and form a consensus view on where best to place our efforts in generating some proof of concept PRs exploring our ideas.

@seothemes
Copy link

What a great reply, thank you for describing the challenges so well.

I missed the single values part in the screen recording, but that's the important bit. Makes me wonder - should negative values only be allowed on individual sides? What would be the use case of having negative margins on all sides at once?

Limiting negative margins to individual sides would help prevent users from breaking layouts.

If blocks are to overlap, should we also be offering control over z-index as well?

I don't think z-index is necessary, and would likely overcomplicate things. We don't need to allow for every possible layout, just the basics. I feel like z-index is closer to positioning, and can quickly get out of control.

We have z-index controls in Blockify for example, which feels like overkill:

positioning controls in blockify

Personally, this gives me further pause towards adding another way to break layouts.

Haha, very good point, and that's the right perspective to have.

would limiting it to themes limit the potential conflicts

Yes, and this would still solve the problem. Themes and plugins should be allowed to do this, whether or not it's added to core. It's just very hard to change and extend from the outside because it's hardcoded.

If added to core, maybe require theme support in theme.json spacing settings to begin with? This would give time to refine and improve the UI:

{
  "settings": {
    "spacing": {
      "negativeMargin": true
    }
  }
}

The new spacing scale does allow for negative values, but that's also applied to padding, which means it's not an option 😢 (#32644 (comment))

My understanding of the general approach in Gutenberg is to try and avoid situations where users are setting values and nothing changes

I'm not sure how other page builders handle situations where the value has no effect, this goes back to negative margins being a CSS issue, which can't really be solved.

Here's how it's done in Webflow for reference:

webflow negative margin control

I guess this topic is bigger than just negative margins. Is the direction of WordPress heading towards no-code, or will it always be "half-code", with the gaps filled by themes and plugins? There seems to be no clear roadmap or intention, unless I'm missing something.

@aaronrobertshaw
Copy link
Contributor

should negative values only be allowed on individual sides?

It may turn out that this would be desired. If negative margins were only allowed for individual sides, we'd need some UI updates regardless. For example, how do we communicate there are different min/max constraints when the control is link/unlinked? How do we avoid confusion and frustration if the user does try to apply a negative margin to all sides?

What would be the use case of having negative margins on all sides at once?

I'm not sure, however, if we are to solve the issues with negative margins for individual sides, I'd guess we'd be in a reasonable place to allow it for all sides.

Themes and plugins should be allowed to do this, whether or not it's added to core

It might be worth noting that negative values are allowed within theme.json. This, of course, only helps when styling all of a given block type. If we are needing to allow configuration of margins for individual blocks via the editor, then I'm not sure we can limit it to theme authors after all. This would bring us back to a theme supplying a custom class and CSS to satisfy their specific needs.

	...
	"styles": {
		"blocks": {
			"core/group": {
				"spacing": {
					"margin": {
						"top": "-100px"
					}
				}
			}
		}
	}

I guess this topic is bigger than just negative margins.

It is in some ways and why I believe it was wise to revert this PR and take a moment to work through the issues before we end up having to provide ongoing support to a potentially broken feature.

In the lead-up to the 6.1 release, I don't have the bandwidth at the moment to wrangle such a complex topic but if you wish to get some further traction on this, I'd recommend creating a new discussion so broader input and feedback can happen.

There seems to be no clear roadmap or intention, unless I'm missing something.

There are a number of resources available detailing a roadmap for Gutenberg and WordPress. Whether that is from a long-term view or for the next release. Here are a few you might find interesting:

@seothemes
Copy link

Thanks again for the very helpful info. Definitely feeling ya on the bandwidth, it's a lot. Appreciate the energy you've already put in.

I spent some time on this and came up with a workaround to allow negative margins:

negative-margin-gutenberg.mov

It's not perfect, negative values need to be entered manually, but that's good in a way. The option is there for designers/developers, but also hidden for beginners. However after doing some testing, I found that negative margins aren't that helpful when it comes to the block editor. Without responsive controls, there's not much that they can be used for.

I'm not sure negative margins are the answer. A much better solution would be for core to add a CSS grid block with responsive settings. That would remove the need for negative margins and many other things. Here's a great example https://wordpress.org/plugins/grids/

Below is the code that I'm using for testing negative margins if anyone is interested:

import { createHigherOrderComponent } from '@wordpress/compose';
import { addFilter } from '@wordpress/hooks';

addFilter(
	'editor.BlockEdit',
	'blockify/with-negative-margin',
	createHigherOrderComponent( BlockEdit => {
		return ( props: blockProps ) => {
			if ( supportsNegativeMargin( props?.name ) ) {
				const input = document.querySelector( '.components-input-control__input[min="0"]' );

				if ( input ) {
					input.setAttribute( 'min', '-999' );
				}
			}

			return <BlockEdit { ...props } />;
		};
	}, 'withMinHeightSettings' )
);

@nicmare
Copy link

nicmare commented Sep 28, 2022

Thanks again for the very helpful info. Definitely feeling ya on the bandwidth, it's a lot. Appreciate the energy you've already put in.

looks different. not the default gutenberg editor shipped with wp 6.0.2 ?!

@seothemes
Copy link

@nicmare not sure which version this was on but it still works with 6.1

@nicmare
Copy link

nicmare commented Nov 21, 2022

@nicmare not sure which version this was on but it still works with 6.1

meanwhile with latest wp i have the new interface and can insert negative values which are reflecting in the editor. But when i hit enter or push save button, it gets reset to zero again :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Negative margin in dimension controls
7 participants