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

[Heading Block] Fix double alignment controls in toolbar #26492

Merged
merged 10 commits into from
Oct 28, 2020

Conversation

ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Oct 27, 2020

Description

Fixes: #26335

After #25917 was merged, the block alignment toolbar is automatically added to the Heading block toolbar, and it controls the align attribute. This attribute was already in use for text alignment, so now there are two controls which both control align.

This PR renames the previous align attribute used for text alignment to textAlign and handles the deprecated block.

Steps to reproduce are on the issue. Basically just add a heading block and try to add both text-align and block-align from its Toolbar.

How has this been tested?

  1. Create some headings from master with different alignments, both text and block.
  2. In this branch go to edit the same post and see that there are no errors and the migration works.
  3. Also test that both alignments are supported now.

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.

@ntsekouras ntsekouras added [Type] Bug An existing feature does not function as intended [Block] Heading Affects the Headings Block Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Oct 27, 2020
@ntsekouras ntsekouras self-assigned this Oct 27, 2020
@github-actions
Copy link

github-actions bot commented Oct 27, 2020

Size Change: +4.24 kB (0%)

Total Size: 1.2 MB

Filename Size Change
build/annotations/index.js 3.78 kB +245 B (6%) 🔍
build/api-fetch/index.js 3.45 kB +106 B (3%)
build/autop/index.js 2.84 kB +112 B (3%)
build/blob/index.js 665 B +1 B
build/block-directory/index.js 8.72 kB +146 B (1%)
build/block-editor/index.js 130 kB -208 B (0%)
build/block-library/index.js 146 kB +577 B (0%)
build/block-serialization-default-parser/index.js 1.88 kB +98 B (5%) 🔍
build/blocks/index.js 48.1 kB +471 B (0%)
build/components/index.js 172 kB -145 B (0%)
build/compose/index.js 9.81 kB +191 B (1%)
build/core-data/index.js 12.3 kB +167 B (1%)
build/data-controls/index.js 675 B -4 B (0%)
build/data/index.js 8.77 kB +155 B (1%)
build/deprecated/index.js 768 B -1 B
build/dom/index.js 4.46 kB +36 B (0%)
build/edit-navigation/index.js 11.2 kB +448 B (4%)
build/edit-post/index.js 306 kB -574 B (0%)
build/edit-site/index.js 21.9 kB -286 B (1%)
build/edit-widgets/index.js 26.4 kB -184 B (0%)
build/editor/index.js 43.1 kB +497 B (1%)
build/element/index.js 4.65 kB +208 B (4%)
build/format-library/index.js 7.7 kB +232 B (3%)
build/hooks/index.js 2.13 kB +390 B (18%) ⚠️
build/i18n/index.js 3.57 kB +21 B (0%)
build/keyboard-shortcuts/index.js 2.52 kB +137 B (5%) 🔍
build/keycodes/index.js 1.94 kB +94 B (4%)
build/list-reusable-blocks/index.js 3.11 kB +102 B (3%)
build/media-utils/index.js 5.34 kB +232 B (4%)
build/notices/index.js 1.79 kB +97 B (5%) 🔍
build/nux/index.js 3.42 kB +158 B (4%)
build/plugins/index.js 2.56 kB +120 B (4%)
build/primitives/index.js 1.43 kB +76 B (5%) 🔍
build/priority-queue/index.js 791 B +1 B
build/redux-routine/index.js 2.84 kB -7 B (0%)
build/reusable-blocks/index.js 3.06 kB +1 B
build/rich-text/index.js 13.2 kB +219 B (1%)
build/server-side-render/index.js 2.77 kB +172 B (6%) 🔍
build/token-list/index.js 1.27 kB +34 B (2%)
build/url/index.js 4.06 kB +7 B (0%)
build/viewport/index.js 1.84 kB +96 B (5%) 🔍
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 11 kB 0 B
build/block-library/editor-rtl.css 8.93 kB 0 B
build/block-library/editor.css 8.93 kB 0 B
build/block-library/style-rtl.css 7.75 kB 0 B
build/block-library/style.css 7.75 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/date/index.js 31.8 kB 0 B
build/dom-ready/index.js 571 B 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/style-rtl.css 6.37 kB 0 B
build/edit-post/style.css 6.35 kB 0 B
build/edit-site/style-rtl.css 3.79 kB 0 B
build/edit-site/style.css 3.79 kB 0 B
build/edit-widgets/style-rtl.css 3.09 kB 0 B
build/edit-widgets/style.css 3.09 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/html-entities/index.js 623 B 0 B
build/is-shallow-equal/index.js 712 B 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/shortcode/index.js 1.69 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@ntsekouras ntsekouras marked this pull request as draft October 27, 2020 11:32
@ntsekouras ntsekouras marked this pull request as ready for review October 27, 2020 12:26
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Great work, I left some nitpicks to look at before merging but overall it resolves the issue with legacy usage of the align attribute for this block.

packages/block-library/src/heading/deprecated.js Outdated Show resolved Hide resolved
packages/block-library/src/heading/deprecated.js Outdated Show resolved Hide resolved
packages/block-library/src/heading/deprecated.js Outdated Show resolved Hide resolved
@ntsekouras ntsekouras merged commit 467175e into master Oct 28, 2020
@ntsekouras ntsekouras deleted the fix/heading-aligns branch October 28, 2020 08:40
@github-actions github-actions bot added this to the Gutenberg 9.3 milestone Oct 28, 2020
tellthemachines pushed a commit that referenced this pull request Oct 30, 2020
* fix heading alignments

* add proper supports in deprecation

* update all previous deprecations

* regenerate fixtures for heading

* change migration call

* remove previous className + revert saves

* Revert "regenerate fixtures for heading"

This reverts commit 27af8c3.

* change fixtures

* create new fixtures + fix deprecation save

* address review feedback
@tellthemachines tellthemachines mentioned this pull request Oct 30, 2020
6 tasks
tellthemachines added a commit that referenced this pull request Nov 2, 2020
* nest content styles in container for higher specificity (#26487)

* Reusable Blocks: Make the number retrieved from the API unlimited (#26486)

* Make the number of reusable blocks returned from the API unlimited

This is to fix #26352. Having no limit seems like it could be risky, but
there are a number of other places in the codebase where all entities
are returned in one call to the API, including categories, which could
return a similar amount of data to this.

* Remove unnecessary quotes to fix lint error.

* Fix block inserter WSOD when an empty reusable block exists (#26484)

* Latest Posts: Bring back classname on post list (#26477)

* [Heading Block] Fix double alignment controls in toolbar (#26492)

* fix heading alignments

* add proper supports in deprecation

* update all previous deprecations

* regenerate fixtures for heading

* change migration call

* remove previous className + revert saves

* Revert "regenerate fixtures for heading"

This reverts commit 27af8c3.

* change fixtures

* create new fixtures + fix deprecation save

* address review feedback

* URLInput: Use debounce() instead of throttle() (#26529)

Wait until the user finishes typing before sending an AJAX request. This
ensures that there isn't an AJAX request sent every 200 ms while the
user is typing.

* Fix single column block display for smaller screens. (#26438)

If there is only one column, don't force a 50% flex-basis for small screens.

* Fix incorrectly pluralised strings (#26565)

* Change block mover label i18n

* Update remove block i18n

* Ensure footer remains position fixed when navigating regions (#26533)

* Update package-lock file to ensure static analysis task passes (#26528)

* Removes extra fullstop (#26586)

Co-authored-by: Addison Stavlo <Stavz01@gmail.com>
Co-authored-by: Paul Bunkham <paul@dobit.co.uk>
Co-authored-by: Noah Allen <noahtallen@gmail.com>
Co-authored-by: Kelly Dwan <ryelle@users.noreply.github.com>
Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
Co-authored-by: Robert Anderson <robert@noisysocks.com>
Co-authored-by: Aaron D. Campbell <aaron@AaronDCampbell.com>
Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
Co-authored-by: Tammie Lister <tammie@automattic.com>
@tellthemachines tellthemachines removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Heading Affects the Headings Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Heading Block: Double alignment controls in toolbar
3 participants