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

Block: move align wrapper out of Block element #23089

Merged
merged 2 commits into from
Jun 11, 2020
Merged

Conversation

ellatrix
Copy link
Member

Description

This PR moves the alignment wrapper out of the Block element. It doesn't really belong in this component, which is meant to render a single element.

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • 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.

@ellatrix ellatrix requested a review from youknowriad as a code owner June 11, 2020 12:32
@ellatrix
Copy link
Member Author

This wrapper should ideally be added with a hook on BlockEdit, together with all the other alignment hooks, but currently that's not possible because alignment support is not explicit in some blocks.

@github-actions
Copy link

github-actions bot commented Jun 11, 2020

Size Change: -3 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-editor/index.js 106 kB -3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.77 kB 0 B
build/block-directory/style-rtl.css 892 B 0 B
build/block-directory/style.css 892 B 0 B
build/block-editor/style-rtl.css 12.1 kB 0 B
build/block-editor/style.css 12.1 kB 0 B
build/block-library/editor-rtl.css 7.88 kB 0 B
build/block-library/editor.css 7.89 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 7.96 kB 0 B
build/block-library/style.css 7.96 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 19.5 kB 0 B
build/components/style.css 19.5 kB 0 B
build/compose/index.js 9.31 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 8.26 kB 0 B
build/edit-navigation/style-rtl.css 975 B 0 B
build/edit-navigation/style.css 974 B 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.6 kB 0 B
build/edit-post/style.css 5.6 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/index.js 9.34 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 423 B 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@ellatrix ellatrix merged commit 44f4054 into master Jun 11, 2020
@ellatrix ellatrix deleted the try/move-align-wrapper branch June 11, 2020 17:14
@github-actions github-actions bot added this to the Gutenberg 8.4 milestone Jun 11, 2020
@ZebulanStanphill ZebulanStanphill added [Package] Block editor /packages/block-editor [Type] Code Quality Issues or PRs that relate to code quality labels Jun 12, 2020
This was referenced Jun 24, 2020
@ockham ockham mentioned this pull request Jun 25, 2020
6 tasks
@keithdevon
Copy link

Hi @ellatrix

I recently submitted an issue about editor styling and after some digging I found that it relates to this PR.

I wondered if I could get some context on why the alignment data were placed in a wrapper rather than on the block element itself?

Is the original reason for this PR still valid, or is there a possibility of reversing this to make styling the editor and front-end more consistent?

Thanks!

@ellatrix
Copy link
Member Author

ellatrix commented Jul 8, 2021

The alignment data was already placed on a wrapper before this PR. This PR just moved the code. See

https://github.com/WordPress/gutenberg/pull/23089/files#diff-ab29c373b565c9af767fdd12831777d155455cee3d6f439c7ae23a75cc030df4L233

The alignment wrapper remains needed I believe for left and right alignment, otherwise it breaks out of the main column. Not sure if we can use the new layout/alignment API here in the future. Cc @youknowriad @jasmussen.

@jasmussen
Copy link
Contributor

jasmussen commented Jul 8, 2021

I would love any way we could revisit that separate wrapper and move closer to the frontend markup on this one. Can we do it for block themes? Already now in block themes, floats break out of the main column.

@youknowriad
Copy link
Contributor

youknowriad commented Jul 8, 2021

the alignment wrapper is not needed for themes with layout support but it's probably still there to support older themes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block editor /packages/block-editor [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants