EditorRegions: Move to block-editor package, rename to __experimentalEditorSkeleton #20050

ockham · 2020-02-05T14:52:25Z

Description

Move the EditorRegions component from packages/post-editor to packages/block-editor, and rename to __experimentalEditorSkeleton. This component is generic enough that it might come in handy for other consumers -- e.g. packages/edit-site, and 3rd parties. (The specifically intended use case will be a 'custom' build of Gutenberg, with some added UI controls etc.) If this is approved, I hope to continue to extract other reusable components similarly.

How has this been tested?

Rebuild Gutenberg (npm run build), and start it (npx wp-env start). Verify that the editor styling isn't broken (specifically regions like header or sidebar).

Types of changes

Move a component from one package to another. Update style selectors, but retain old class names along new ones for backwards compat.

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.

gziolo · 2020-02-05T16:19:03Z

@youknowriad is your friend, he has a better idea of why this component is in @wordpress/edit-post. I think this component is quite opinionated and I can't tell if it's applicable for edit-site or edit-widgets. However, I understand the point that other projects would like to reuse it to keep the same structure of the content. The only challenge is that kind of components might change the order of items if there is a need to improve flows.

packages/editor/src/components/editor-regions/index.js

youknowriad · 2020-02-06T09:27:14Z

packages/editor/src/components/editor-regions/index.js

 			{ !! header && (
 				<div
-					className="edit-post-editor-regions__header"
+					className="components-editor-regions__header edit-post-editor-regions__header"
 					role="region"
 					/* translators: accessibility text for the top bar landmark region. */
 					aria-label={ __( 'Editor top bar' ) }


I was wondering whether the aria-label are too opinionated and should be made customizable or not. Potentially, they couuld work for any "editor".

Yeah, maybe 🤔 I'd go with YAGNI here -- don't make it customizable until needed?

packages/editor/src/components/editor-regions/index.js

ockham · 2020-02-06T14:34:44Z

I moved the component to the block-editor package, and renamed it to __experimentalEditorSkeleton. @youknowriad Care to take another look? 😊

youknowriad · 2020-02-06T15:05:53Z

packages/block-editor/src/components/editor-skeleton/index.js

+			className={ classnames(
+				className,
+				'block-editor-editor-skeleton',
+				'edit-post-editor-regions'


I think we can drop this className tbh. Did you leave it for BC? This seems to be ready to land before beta1 so there shouldn't be any problem to remove the classname.

Yeah, I left it in for backwards compat. I know of at least one instance in Calypso where we're referring to this class name, and since we were having quite some trouble with class name changes in the past (specifically with the .editor- -> .block-editor- change, see Automattic/wp-calypso#38857, #14420, and #19050), I wanted at least to give a grace period before dropping the old class name.

(Then again, a point can be made that this was never part of a public interface, and that that grace period didn't help in updating the affected Calypso selectors in time before they were dropped in the past either, so 🤷‍♂️ )

youknowriad · 2020-02-06T15:06:51Z

As part of this PR? 🤔 Or fine to do it in follow-up?

Whatever you think is best, that said, I think it's important to validate that this component is generic enough to be shared properly.

youknowriad

Thanks 👍

ockham · 2020-02-06T17:41:41Z

As part of this PR? thinking Or fine to do it in follow-up?

Whatever you think is best, that said, I think it's important to validate that this component is generic enough to be shared properly.

I'll file a follow-up; if there are any changes needed to EditorSkeleton, I'll make them there.

youknowriad · 2020-02-07T09:09:45Z

you can merge since it's approved. I figured you might be waiting for me?

ockham · 2020-02-10T07:14:07Z

you can merge since it's approved. I figured you might be waiting for me?

Thanks for merging! I was considering dropping the BC stuff we discussed here as part of this PR but didn't get to that on Friday 😅

WordPress/gutenberg#20050

* Update `@wordpress/*` dependencies * Add externalized dependencies to FSE package.json * Update z-index classname WordPress/gutenberg#20050 * Add webpack alias to deduplicate `@wordpress/data` * Remove selected block outlines Co-authored-by: WhiteSource Renovate <renovatebot@gmail.com> Co-authored-by: Jon Surrell <jon.surrell@automattic.com>

ockham added 6 commits February 5, 2020 20:14

EditorRegions: Move to editor package

bd547d4

Move style import

3410c14

Update EditorRegions import

ee0ec08

Export EditorRegions from editor package

384aec4

s/edit-post-editor-regions/components-editor-regions/g

eeac542

Add new class names to EditorRegions component

d3bbbed

ockham added the [Package] Editor /packages/editor label Feb 5, 2020

ockham requested review from gziolo, vindl and epiqueras February 5, 2020 14:52

ockham requested review from ajitbohra, nerrad, ntwb and talldan as code owners February 5, 2020 14:52

ockham self-assigned this Feb 5, 2020

gziolo requested a review from youknowriad February 5, 2020 16:15

youknowriad reviewed Feb 6, 2020

View reviewed changes

youknowriad mentioned this pull request Feb 6, 2020

Resizable editor experiment #19082

Merged

6 tasks

ockham added 6 commits February 6, 2020 18:34

Move to block-editor

b40d6ce

Fix exports/imports

90a0657

s/components-editor-regions/block-editor-editor-regions/g

827b450

Change classnames to block-editor-editor-skeleton*

953c8b0

Rename to EditorSkeleton

810cf80

Prefix as __experimental

5df7314

ockham requested a review from ellatrix as a code owner February 6, 2020 14:31

ockham changed the title ~~EditorRegions: Move to editor package~~ EditorRegions: Move to block-editor package, rename to __experimentalEditorSkeleton Feb 6, 2020

youknowriad reviewed Feb 6, 2020

View reviewed changes

youknowriad approved these changes Feb 6, 2020

View reviewed changes

youknowriad merged commit de7c442 into master Feb 7, 2020

youknowriad deleted the move/editor-scaffolding-to-editor-package branch February 7, 2020 09:10

github-actions bot added this to the Gutenberg 7.5 milestone Feb 7, 2020

ockham mentioned this pull request Feb 10, 2020

EditorSkeleton: Add Changelog entries, drop backwards-compat class names #20132

Merged

4 tasks

jorgefilipecosta mentioned this pull request Feb 14, 2020

Sites unscrollable on mobile after 7.5.0 update #20214

Closed

sirreal mentioned this pull request Feb 18, 2020

fix(deps): update wordpress monorepo Automattic/wp-calypso#39383

Merged

1 task

sirreal added a commit to Automattic/wp-calypso that referenced this pull request Feb 18, 2020

Update z-index classname

a29ff04

WordPress/gutenberg#20050

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EditorRegions: Move to block-editor package, rename to __experimentalEditorSkeleton #20050

EditorRegions: Move to block-editor package, rename to __experimentalEditorSkeleton #20050

ockham commented Feb 5, 2020 •

edited

Loading

gziolo commented Feb 5, 2020

youknowriad Feb 6, 2020

ockham Feb 6, 2020

ockham commented Feb 6, 2020

youknowriad Feb 6, 2020

ockham Feb 6, 2020

youknowriad commented Feb 6, 2020

youknowriad left a comment

ockham commented Feb 6, 2020

youknowriad commented Feb 7, 2020 •

edited

Loading

ockham commented Feb 10, 2020

EditorRegions: Move to block-editor package, rename to __experimentalEditorSkeleton #20050

EditorRegions: Move to block-editor package, rename to __experimentalEditorSkeleton #20050

Conversation

ockham commented Feb 5, 2020 • edited Loading

Description

How has this been tested?

Types of changes

Checklist:

gziolo commented Feb 5, 2020

youknowriad Feb 6, 2020

Choose a reason for hiding this comment

ockham Feb 6, 2020

Choose a reason for hiding this comment

ockham commented Feb 6, 2020

youknowriad Feb 6, 2020

Choose a reason for hiding this comment

ockham Feb 6, 2020

Choose a reason for hiding this comment

youknowriad commented Feb 6, 2020

youknowriad left a comment

Choose a reason for hiding this comment

ockham commented Feb 6, 2020

youknowriad commented Feb 7, 2020 • edited Loading

ockham commented Feb 10, 2020

ockham commented Feb 5, 2020 •

edited

Loading

youknowriad commented Feb 7, 2020 •

edited

Loading