Skip to content

/ gutenberg

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

Jump to bottom

Inline font-sizes: avoid modifying the BlockListBlock props object #23717

Merged
merged 3 commits into from Jul 6, 2020
Merged

Inline font-sizes: avoid modifying the BlockListBlock props object #23717

merged 3 commits into from Jul 6, 2020

Conversation

@Copons
Copy link
Contributor

@Copons Copons commented Jul 6, 2020

Description

  • Avoid changing the BlockListBlock props object in the withFontSizeInlineStyles HOC.
  • Also fix a typo in the useFontSizes hook name.

How has this been tested?

Tested with wp-env on both Firefox 78 and Chrome 83 on macOS 10.15.5.
The issue appeared in the Site Editor while editing the home template of the theme Seedlet (Blocks).

Screenshots

Screenshot 2020-07-06 at 16 01 05

This screenshot was captured while testing the Site Editor.
Both Section blocks were breaking because of an Uncaught TypeError: can't define property "wrapperProps": Object is not extensible, thrown by this line:

https://github.com/WordPress/gutenberg/pull/22668/files#diff-7e9533138d2d1bd1c8f0f4b32d2d3aa4R189

Types of changes

Bug fix

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.
Copons added 2 commits Jul 6, 2020
@Copons
Avoid changing the BlockListBlock props object
e108170
@Copons
Fix typo
Loading status checks…
fc427f2
@Copons Copons requested a review from nosolosw Jul 6, 2020
@Copons Copons requested review from ellatrix and youknowriad as code owners Jul 6, 2020
@Copons Copons self-assigned this Jul 6, 2020
@vindl vindl requested review from ockham, noahtallen, vindl and Addison-Stavlo Jul 6, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Jul 6, 2020 •
edited

Size Change: +8 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-editor/index.js 110 kB +8 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.68 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.67 kB 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 7.57 kB 0 B
build/block-library/editor.css 7.57 kB 0 B
build/block-library/index.js 130 kB 0 B
build/block-library/style-rtl.css 7.78 kB 0 B
build/block-library/style.css 7.79 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 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.2 kB 0 B
build/components/index.js 199 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.8 kB 0 B
build/compose/index.js 9.56 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.38 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/index.js 9.92 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.57 kB 0 B
build/edit-post/style.css 5.57 kB 0 B
build/edit-site/index.js 16.7 kB 0 B
build/edit-site/style-rtl.css 3.03 kB 0 B
build/edit-site/style.css 3.03 kB 0 B
build/edit-widgets/index.js 9.35 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45 kB 0 B
build/editor/style-rtl.css 3.77 kB 0 B
build/editor/style.css 3.77 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.73 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 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.52 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 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.41 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.71 kB 0 B
build/shortcode/index.js 1.69 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
@ockham ockham mentioned this pull request Jul 6, 2020
Merged
@youknowriad
youknowriad reviewed Jul 6, 2020
View changes
packages/block-editor/src/hooks/font-size.js Outdated
},
};

return <BlockListBlock { ...newProps } />;

This comment has been minimized.

Sign in to view
@youknowriad

youknowriad Jul 6, 2020
Contributor

I wonder if returning here will cause "remounting" of the component? Can we instead keep the mutation here but declarer newProps at the top of the component (and spread props)

This comment has been minimized.

Sign in to view
@Copons

Copons Jul 6, 2020
Author Contributor

Done in ae0ba69!
@youknowriad youknowriad added this to the Gutenberg 8.5 milestone Jul 6, 2020
@Copons
Prevent remounting
Loading status checks…
ae0ba69
@youknowriad
youknowriad approved these changes Jul 6, 2020
View changes
Copy link
Contributor

@youknowriad youknowriad left a comment

LGTM 👍
@Copons Copons merged commit 03d4914 into master Jul 6, 2020
13 checks passed
13 checks passed
Check Check
Details
build
Details
Admin - 1
Details
pull-request-automation
Details
test (gutenberg-editor-gallery)
Details
test (gutenberg-editor-gallery)
Details
All
Details
JavaScript
Details
Admin - 2
Details
PHP
Details
Admin - 3
Details
Mobile
Details
Admin - 4
Details
@Copons Copons deleted the fix/font-size-props-object-not-extensible branch Jul 6, 2020
@ellatrix
Copy link
Member

@ellatrix ellatrix commented Jul 6, 2020

Thanks :) I think I was loading the wrong branch...
gziolo added a commit that referenced this pull request Jul 7, 2020
@Copons @gziolo
Inline font-sizes: avoid modifying the BlockListBlock props object (#…
Loading status checks…
8384ca7 
…23717)

- Avoid changing the `BlockListBlock` props object in the `withFontSizeInlineStyles` HOC.
- Also fix a typo in the `useFontSizes` hook name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowriad youknowriad

@ockham ockham

@nosolosw nosolosw

@vindl vindl

@ellatrix ellatrix

@noahtallen noahtallen

@Addison-Stavlo Addison-Stavlo

Assignees

@Copons Copons

Labels
[Type] Bug
Projects
None yet
Milestone
Gutenberg 8.5
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@Copons @ellatrix @youknowriad
You can’t perform that action at this time.