Refactor Embed Edit component: Class component to Function component #22846

desaiuditd · 2020-06-03T01:15:36Z

Description

This refactor stems from #21789 where improvements for Embed block UX are discussed. But before we actually work on improvements, this seemed like a good opportunity to convert the component into a function component so that React hooks can be used within the component later.

How has this been tested?

Tested locally to make sure embed block works properly with all the basic use cases like changing the url, block transformation, switching between preview and placeholder etc.

Types of changes

Code refactoring to convert the edit component of Embed block into a function component. Non-breaking change. No behaviour change is expected.

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.

github-actions · 2020-06-03T01:22:14Z

Size Change: -459 B (0%)

Total Size: 1.13 MB

Filename	Size	Change
`build/block-editor/index.js`	109 kB	-3 B (0%)
`build/block-library/index.js`	129 kB	-452 B (0%)
`build/components/index.js`	198 kB	-1 B
`build/data/index.js`	8.44 kB	+1 B
`build/edit-navigation/index.js`	9.87 kB	+1 B
`build/edit-post/index.js`	303 kB	-1 B
`build/edit-site/index.js`	16.6 kB	-1 B
`build/edit-widgets/index.js`	9.32 kB	-1 B
`build/list-reusable-blocks/index.js`	3.12 kB	-1 B
`build/media-utils/index.js`	5.29 kB	-1 B
`build/plugins/index.js`	2.56 kB	-1 B
`build/token-list/index.js`	1.28 kB	+1 B

ℹ️ 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.82 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	7.38 kB	0 B
`build/block-directory/style-rtl.css`	941 B	0 B
`build/block-directory/style.css`	942 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.6 kB	0 B
`build/block-library/editor.css`	7.6 kB	0 B
`build/block-library/style-rtl.css`	8.04 kB	0 B
`build/block-library/style.css`	8.04 kB	0 B
`build/block-library/theme-rtl.css`	730 B	0 B
`build/block-library/theme.css`	732 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/style-rtl.css`	15.9 kB	0 B
`build/components/style.css`	15.9 kB	0 B
`build/compose/index.js`	9.65 kB	0 B
`build/core-data/index.js`	11.4 kB	0 B
`build/data-controls/index.js`	1.29 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`	569 B	0 B
`build/dom/index.js`	3.19 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/style-rtl.css`	5.51 kB	0 B
`build/edit-post/style.css`	5.5 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/style-rtl.css`	2.42 kB	0 B
`build/edit-widgets/style.css`	2.42 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`	44.8 kB	0 B
`build/editor/style-rtl.css`	3.85 kB	0 B
`build/editor/style.css`	3.85 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.72 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`	711 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/style-rtl.css`	450 B	0 B
`build/list-reusable-blocks/style.css`	451 B	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`	663 B	0 B
`build/nux/style.css`	660 B	0 B
`build/primitives/index.js`	1.5 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`	14 kB	0 B
`build/server-side-render/index.js`	2.68 kB	0 B
`build/shortcode/index.js`	1.7 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}

mcsf · 2020-06-03T09:54:52Z

Taking a quick look, this seems like good work. @ZebulanStanphill, can you lead the review? I would say that one thing to pay special attention to is performance: making sure there aren't clear regressions, and assess whether and where we might benefit from useCallback or useMemo, for instance. Thank you to both of you!

desaiuditd · 2020-06-03T10:45:29Z

Just saw that the e2e tests are failing in CI. Quick look told me that it's relevant. I'll try to fix it. But it will be the first time I'll touch e2e tests. So please bear with me. Thanks 🙂

packages/block-library/src/embed/edit.js

ZebulanStanphill · 2020-06-03T16:23:41Z

packages/block-library/src/embed/edit.js

 import { __, sprintf } from '@wordpress/i18n';
-import { Component } from '@wordpress/element';
+import { useState, useEffect } from '@wordpress/element';


Nitpick: I prefer imports to be in alphabetical order. (There's no standard one way or the other; it's just a personal preference of mine.)

Example:

import { useEffect, useState } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n';

Either let's make a linting rule, or not bother with this at all. :) I personally don't feel like wasting time on this.

mcsf · 2020-06-19T10:51:29Z

@desaiuditd, @ZebulanStanphill: do you see anything missing from this PR, or can we merge?

paaljoachim · 2020-06-21T20:38:30Z

@swissspidy is this PR something you can help out with? As the PR is very slowly moving forward. Thanks.

Convert the Embed Edit component into a function component so that React hooks can be used within the component. * Fixed rebase conflicts with b2ec648 and a610923

Co-authored-by: Zebulan Stanphill <[email protected]>

mcsf · 2020-06-26T14:41:00Z

Rebased against master.

desaiuditd · 2020-06-27T07:42:14Z

@mcsf Thanks. I was away from my dev machine for awhile. I'll take a look at this today.

desaiuditd · 2020-06-28T06:56:46Z

Just checked this. I think this is looking good to me. Ready for review/merge.

I found a bug in the Try Again button in the edit mode. However, I verified that it's happening on master branch as well and bug is coming from

gutenberg/packages/block-library/src/embed/settings.js

Line 111 in b2ec648

withDispatch( ( dispatch, ownProps ) => {

. I can fix it in next PR.

mcsf

Thanks for working on this!

paaljoachim · 2020-06-29T13:17:55Z

What is the overall path forward in regards to the embed feature?

desaiuditd added [Type] Code Quality Issues or PRs that relate to code quality [Block] Embed Affects the Embed Block labels Jun 3, 2020

desaiuditd requested review from ajitbohra, jorgefilipecosta, Soean and talldan as code owners June 3, 2020 01:15

desaiuditd requested review from mcsf and ZebulanStanphill June 3, 2020 01:15

ZebulanStanphill reviewed Jun 3, 2020

View reviewed changes

packages/block-library/src/embed/edit.js Show resolved Hide resolved

ZebulanStanphill reviewed Jun 3, 2020

View reviewed changes

packages/block-library/src/embed/edit.js Outdated Show resolved Hide resolved

ZebulanStanphill reviewed Jun 3, 2020

View reviewed changes

ZebulanStanphill added the [Package] Block library /packages/block-library label Jun 12, 2020

desaiuditd and others added 3 commits June 26, 2020 12:20

Refactor Embed Edit component

9ef6dbe

Convert the Embed Edit component into a function component so that React hooks can be used within the component. * Fixed rebase conflicts with b2ec648 and a610923

Update EmbedEditComponent's effect dependencies

821ef72

Co-authored-by: Zebulan Stanphill <[email protected]>

Fix URL-setting effect dependencies

5444d51

mcsf force-pushed the update/refactor-embed-block-edit-component branch from 86e355c to 5444d51 Compare June 26, 2020 13:45

Merge branch 'master' into update/refactor-embed-block-edit-component

a386df3

mcsf approved these changes Jun 29, 2020

View reviewed changes

mcsf merged commit e5a1d0e into master Jun 29, 2020

mcsf deleted the update/refactor-embed-block-edit-component branch June 29, 2020 14:22

github-actions bot added this to the Gutenberg 8.5 milestone Jun 29, 2020

paaljoachim mentioned this pull request Sep 15, 2020

Embeds: WordPress Embeds not properly previewed in Editor #25344

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Embed Edit component: Class component to Function component #22846

Refactor Embed Edit component: Class component to Function component #22846

desaiuditd commented Jun 3, 2020

github-actions bot commented Jun 3, 2020 •

edited

Loading

mcsf commented Jun 3, 2020

desaiuditd commented Jun 3, 2020

ZebulanStanphill Jun 3, 2020 •

edited

Loading

ellatrix Jun 3, 2020

mcsf commented Jun 19, 2020

paaljoachim commented Jun 21, 2020 •

edited

Loading

mcsf commented Jun 26, 2020

desaiuditd commented Jun 27, 2020

desaiuditd commented Jun 28, 2020 •

edited

Loading

mcsf left a comment

paaljoachim commented Jun 29, 2020

Refactor Embed Edit component: Class component to Function component #22846

Refactor Embed Edit component: Class component to Function component #22846

Conversation

desaiuditd commented Jun 3, 2020

Description

How has this been tested?

Types of changes

Checklist:

github-actions bot commented Jun 3, 2020 • edited Loading

mcsf commented Jun 3, 2020

desaiuditd commented Jun 3, 2020

ZebulanStanphill Jun 3, 2020 • edited Loading

Choose a reason for hiding this comment

ellatrix Jun 3, 2020

Choose a reason for hiding this comment

mcsf commented Jun 19, 2020

paaljoachim commented Jun 21, 2020 • edited Loading

mcsf commented Jun 26, 2020

desaiuditd commented Jun 27, 2020

desaiuditd commented Jun 28, 2020 • edited Loading

mcsf left a comment

Choose a reason for hiding this comment

paaljoachim commented Jun 29, 2020

github-actions bot commented Jun 3, 2020 •

edited

Loading

ZebulanStanphill Jun 3, 2020 •

edited

Loading

paaljoachim commented Jun 21, 2020 •

edited

Loading

desaiuditd commented Jun 28, 2020 •

edited

Loading