Skip to content

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

Merged
Copons merged 3 commits intomasterfrom
fix/font-size-props-object-not-extensible
Jul 6, 2020
Merged

Inline font-sizes: avoid modifying the BlockListBlock props object#23717
Copons merged 3 commits intomasterfrom
fix/font-size-props-object-not-extensible

Conversation

@Copons
Copy link
Copy Markdown
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 Copons added the [Type] Bug An existing feature does not function as intended label Jul 6, 2020
@Copons Copons requested a review from oandregal July 6, 2020 15:07
@Copons Copons self-assigned this Jul 6, 2020
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 6, 2020

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

},
};

return <BlockListBlock { ...newProps } />;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in ae0ba69!

@youknowriad youknowriad added this to the Gutenberg 8.5 milestone Jul 6, 2020
Copy link
Copy Markdown
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Copons Copons merged commit 03d4914 into master Jul 6, 2020
@Copons Copons deleted the fix/font-size-props-object-not-extensible branch July 6, 2020 16:16
@ellatrix
Copy link
Copy Markdown
Member

ellatrix commented Jul 6, 2020

Thanks :) I think I was loading the wrong branch...

gziolo pushed a commit that referenced this pull request Jul 7, 2020
…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

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants