Skip to content
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

Edit Site: Add package with barebones site editor screen. #19054

Merged
merged 4 commits into from
Dec 16, 2019

Conversation

epiqueras
Copy link
Contributor

Description

This PR starts work on an experimental block based full site editing screen as part of the full site editing experiment.

The idea is for this to serve as a specialized UI for customizing block based themes, basically what the customizer is now to classic themes.

This PR takes care of adding the new module, linking all necessary dependencies, bootstrapping PHP logic, etc. It's already pretty big as is, so it'd be good to merge this before starting to work on the actual full site editing logic.

How has this been tested?

It was verified that enabling the full site editing experiment in the Gutenberg experiments screen enabled a "Site editor (beta)" sub-menu item in the Gutenberg menu, and clicking on it rendered a basic block editor without persistence.

Screenshots

Screen Shot 2019-12-10 at 4 37 43 PM

Types of Changes

New Feature: The full site editing experiment now has an experimental site editor screen that will be developed for customizing block based themes.

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. .

$screen->is_block_editor() ||
'gutenberg_page_gutenberg-widgets' === $screen->id ||
'gutenberg_page_gutenberg-edit-site' === $screen->id
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this means this function should move out of lib/widgets.php

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, it's still only needed for widget blocks.

to {
opacity: 1;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

One of the reasons of the introduction of the EditorRegions component was to abstract the layout of the editor page to use it elsewhere instead of duplicating all these stylesheets. It could be an opportunity to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I thought/tried that. The problem is that it's still very specific to edit-post, we would have to move it to block-editor and make it a bit more abstract maybe?

I think that it won't take long before the edit-post and edit-site layouts diverge so much that it will become impractical to share something like EditorRegions.

I do see potential for something similar to EditorRegions, but specific to edit-site, later down the road when the desired layout is clearer.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's specific to edit-post in that package is just the aria regions labels. Other than that I think it's generic enough. Tbh, I feel we should do the refactoring now rather than after the fact.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move EditorRegions to editor and use it here? I don't think that will be harder to do later, but we could do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure yet which package, could be block-editor maybe? I don't know yet since it's basically just a UI component but maybe not generic enough for @wordpress/components?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems more like an editor thing since it deals with the area that surrounds the block-editor.

Copy link
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.

It's still not clear to me whether this new edit-site package should be a complete new React tree or just reuse wp.editPost.initializeEditor and just enable/disable some plugins/hooks.

I guess this comes down to the size of the differences between the two screens. It will become obvious over time, so happy to start with the proposed approach here.

@epiqueras
Copy link
Contributor Author

I think that they will be very different and we would end up polluting edit-post with lots of awkward extensibility points.

@epiqueras epiqueras mentioned this pull request Dec 12, 2019
6 tasks
@epiqueras epiqueras requested review from mtias and aduth December 12, 2019 01:02
@epiqueras epiqueras force-pushed the add/edit-site-package branch from 0abc8dd to 9590317 Compare December 12, 2019 23:13
Copy link
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.

I'm approving but I'd like us to try extracting the EditorRegions component to reduce the boilerplate.

@epiqueras epiqueras merged commit d3d1747 into master Dec 16, 2019
@youknowriad youknowriad modified the milestones: Future, Gutenberg 7.2 Jan 6, 2020
@aristath aristath deleted the add/edit-site-package branch November 10, 2020 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants