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:42Z

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 [Block] Embed labels Jun 3, 2020

desaiuditd requested review from ajitbohra, jorgefilipecosta, Soean and talldan as code owners Jun 3, 2020

desaiuditd requested review from mcsf and ZebulanStanphill Jun 3, 2020

ZebulanStanphill reviewed Jun 3, 2020

View changes

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

ZebulanStanphill reviewed Jun 3, 2020

View changes

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

ZebulanStanphill reviewed Jun 3, 2020

View changes

ZebulanStanphill added the [Package] Block library label Jun 12, 2020

desaiuditd and others added 3 commits May 30, 2020

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

Loading status checks…

5444d51

mcsf force-pushed the update/refactor-embed-block-edit-component branch from 86e355c to 5444d51 Jun 26, 2020

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

Loading status checks…

a386df3

mcsf approved these changes Jun 29, 2020

View changes

mcsf deleted the update/refactor-embed-block-edit-component branch Jun 29, 2020

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

WordPress / gutenberg

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

mcsf commented Jun 3, 2020

desaiuditd commented Jun 3, 2020

This comment has been minimized.

This comment has been minimized.

mcsf commented Jun 19, 2020

paaljoachim commented Jun 21, 2020 •

edited

mcsf commented Jun 26, 2020

desaiuditd commented Jun 27, 2020

desaiuditd commented Jun 28, 2020 •

edited

mcsf left a comment

paaljoachim commented Jun 29, 2020

WordPress / gutenberg

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

mcsf commented Jun 3, 2020

desaiuditd commented Jun 3, 2020

This comment has been minimized.

ZebulanStanphill Jun 3, 2020 • edited Member

This comment has been minimized.

ellatrix Jun 3, 2020 Member

mcsf commented Jun 19, 2020

paaljoachim commented Jun 21, 2020 • edited

mcsf commented Jun 26, 2020

desaiuditd commented Jun 27, 2020

desaiuditd commented Jun 28, 2020 • edited

mcsf left a comment

paaljoachim commented Jun 29, 2020

github-actions bot commented Jun 3, 2020 •

edited

ZebulanStanphill Jun 3, 2020 •

edited

Member

ellatrix Jun 3, 2020
Member

paaljoachim commented Jun 21, 2020 •

edited

desaiuditd commented Jun 28, 2020 •

edited