Add: Plugin Sidebar management functionality to plugins package. #20336

jorgefilipecosta · 2020-02-20T15:55:57Z

This PR adds Plugin Sidebar management functionality to plugins package similar to what we had in edit post.

That functionality is used to implement sidebars in edit-site uses the functionality on edit-site.

In order to make this PR reviewable for now, just PluginSidebar is added to plugins.

If we agree with this approach the following follow-ups are required:

We will also need to expose functionality to render menu items to open the sidebar when it is not pinned.
We need to add the UI to pin and unpin sidebar items.
We need to refactor edit-post to use this functionality instead of using its own.

The plugins API is used in edit-site to register a sidebar for the block inspector and for the global styles.

cc: @ItsJonQ, @gziolo

How has this been tested?

I went to edit site and I verified I could toggle the block inspector and the "global styles sidebar" for now just a paragraph of text.

gziolo · 2020-02-20T16:19:13Z

@jorgefilipecosta - can you elaborate more about the components and APIs introduced? Names differ from edit-post uses so I have trouble connecting the dots. Maybe you could add a screencast or screenshot that emphasizes new behavior added.

github-actions · 2020-02-20T18:40:04Z

Size Change: +2.52 kB (0%)

Total Size: 864 kB

Filename	Size	Change
`build/annotations/index.js`	3.43 kB	+3 B (0%)
`build/api-fetch/index.js`	3.39 kB	+1 B
`build/autop/index.js`	2.59 kB	+1 B
`build/block-directory/index.js`	6.02 kB	+1 B
`build/block-editor/index.js`	104 kB	+17 B (0%)
`build/block-library/index.js`	114 kB	+23 B (0%)
`build/blocks/index.js`	57.6 kB	+3 B (0%)
`build/components/index.js`	190 kB	+2 B (0%)
`build/compose/index.js`	5.75 kB	-3 B (0%)
`build/data-controls/index.js`	1.04 kB	-1 B
`build/data/index.js`	8.22 kB	-8 B (0%)
`build/dom/index.js`	3.06 kB	-1 B
`build/edit-post/index.js`	90.8 kB	+105 B (0%)
`build/edit-site/index.js`	3.32 kB	+613 B (18%)	⚠️
`build/edit-site/style-rtl.css`	2.63 kB	+16 B (0%)
`build/edit-site/style.css`	2.63 kB	+16 B (0%)
`build/edit-widgets/index.js`	4.36 kB	+1 B
`build/editor/index.js`	45.2 kB	+30 B (0%)
`build/keyboard-shortcuts/index.js`	2.3 kB	+1 B
`build/notices/index.js`	1.57 kB	+3 B (0%)
`build/nux/index.js`	3.02 kB	+1 B
`build/plugins/index.js`	4.24 kB	+1.69 kB (39%)	🚨
`build/primitives/index.js`	1.49 kB	-1 B
`build/redux-routine/index.js`	2.84 kB	-1 B
`build/rich-text/index.js`	14.3 kB	-1 B
`build/server-side-render/index.js`	2.54 kB	+1 B
`build/token-list/index.js`	1.27 kB	-1 B

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.01 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/style-rtl.css`	760 B	0 B
`build/block-directory/style.css`	760 B	0 B
`build/block-editor/style-rtl.css`	9.78 kB	0 B
`build/block-editor/style.css`	9.77 kB	0 B
`build/block-library/editor-rtl.css`	7.67 kB	0 B
`build/block-library/editor.css`	7.67 kB	0 B
`build/block-library/style-rtl.css`	7.47 kB	0 B
`build/block-library/style.css`	7.48 kB	0 B
`build/block-library/theme-rtl.css`	669 B	0 B
`build/block-library/theme.css`	671 B	0 B
`build/block-serialization-default-parser/index.js`	1.65 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/components/style-rtl.css`	16.1 kB	0 B
`build/components/style.css`	16 kB	0 B
`build/core-data/index.js`	10.5 kB	0 B
`build/date/index.js`	5.36 kB	0 B
`build/deprecated/index.js`	771 B	0 B
`build/dom-ready/index.js`	569 B	0 B
`build/edit-post/style-rtl.css`	8.69 kB	0 B
`build/edit-post/style.css`	8.68 kB	0 B
`build/edit-widgets/style-rtl.css`	2.8 kB	0 B
`build/edit-widgets/style.css`	2.79 kB	0 B
`build/editor/editor-styles-rtl.css`	327 B	0 B
`build/editor/editor-styles.css`	328 B	0 B
`build/editor/style-rtl.css`	4.13 kB	0 B
`build/editor/style.css`	4.11 kB	0 B
`build/element/index.js`	4.45 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.6 kB	0 B
`build/format-library/style-rtl.css`	500 B	0 B
`build/format-library/style.css`	501 B	0 B
`build/hooks/index.js`	1.92 kB	0 B
`build/html-entities/index.js`	621 B	0 B
`build/i18n/index.js`	3.45 kB	0 B
`build/is-shallow-equal/index.js`	711 B	0 B
`build/keycodes/index.js`	1.68 kB	0 B
`build/list-reusable-blocks/index.js`	2.99 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	215 B	0 B
`build/list-reusable-blocks/style.css`	216 B	0 B
`build/media-utils/index.js`	4.84 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/plugins/style-rtl.css`	90 B	0 B
`build/plugins/style.css`	90 B	0 B
`build/priority-queue/index.js`	878 B	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/url/index.js`	4 kB	0 B
`build/viewport/index.js`	1.61 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

…the functionality on edit-site.

jorgefilipecosta · 2020-02-20T20:00:54Z

Hi @gziolo, as an overview of what we are exposing now:
We expose wp.plugins.PluginSidedar that works more or less like wp.editPost.PluginSidedar but supports scopes so it can deal with multiple sidebars.

Under PluginSidedar we have PluginSidedar.Slot that is a slot to render the sidebar and we also have PluginSidedar.PinnedItemsSlot is a slot where the pinned items will be rendered.

We expose an API in the store that allows keeping track of which area is active in a given zone when only one area can be active at a time. For example what sidebar is being rendered, what tab in a tab system is enabled etc...

wp.data.dispatch('core/plugins').setSingleActiveArea( 'edit-site/sidebar', 'block-inspector' );

wp.data.select('core/plugins').getSingleActiveArea( 'edit-site/sidebar' ); -> "block-inspector"

wp.data.dispatch('core/plugins').setSingleActiveArea( 'edit-site/sidebar', 'global-styles' );

wp.data.select('core/plugins').getSingleActiveArea( 'edit-site/sidebar' ); -> "global-styles"

wp.data.dispatch('core/plugins').setSingleActiveArea( 'edit-site/sidebar' );

wp.data.select('core/plugins').getSingleActiveArea( 'edit-site/sidebar' ); -> undefined

We expose an API in the store that allows keeping track of which areas are active in a given zone when multiple areas can be active at a time
E.g: which plugins are pinned, which panels are open.
This API allows plugins to keep track for example which panels are open and take advantage of the persisting mechanism (when we add it) without the need to implement their own store/persistence.

wp.data.select('core/plugins').isMultipleActiveAreaActive( 'edit-site/some-panel', 'panel1' ); -> false

wp.data.dispatch('core/plugins').setMultipleActiveAreaEnableState( 'edit-site/some-panel', 'panel1', true );

wp.data.select('core/plugins').isMultipleActiveAreaActive( 'edit-site/some-panel', 'panel1' ); -> true

gziolo · 2020-02-21T11:21:38Z

One important thing I would like to fix here is that we don’t use the “sidebar” keyword in the @wordpres/plugins package. I was looking at ARIA landmarks to check what would be a good fit and I like the name and role assigned to the complementary region:
https://www.w3.org/TR/wai-aria-practices/examples/landmarks/complementary.html

The way the edit post page is structured prevents us from using complementary landmark:

complementary landmarks should be top level landmarks (e.g. not contained within any other landmarks).

However, when you ignore all the WP-Admin wrapping, this is the best way to think about the sidebar.

How do you feel about using PluginComplementaryArea or something like that?

gziolo · 2020-02-21T13:53:56Z

packages/edit-site/src/components/header/index.js

@@ -21,6 +22,7 @@ export default function Header() {
 			</h1>
 			<div className="edit-site-header__actions">
 				<SaveButton />
+				<PluginSidebar.PinnedItemsSlot scope="edit-site/sidebar" />


I don't think that PinnedItems should be specific to the sidebar. It's a completely independent feature that provides shortcuts to access plugins. The current implementation might be misleading:

gutenberg/packages/edit-post/src/components/sidebar/plugin-sidebar/index.js

Lines 34 to 46 in ec68bb8

{ isPinnable && (

<PinnedPlugins>

{ isPinned && (

<Button

icon={ icon }

label={ title }

onClick={ toggleSidebar }

isPressed={ isActive }

aria-expanded={ isActive }

/>

) }

</PinnedPlugins>

) }

It's colocated with the Sidebar component that has this button that allows to pin or unpin it. However, you could easily make the menu item that executes an action pinnable. Everything can be pinnable. There is probably a lot of prior knowledge hidden on GitHub accumulated in the discussions between me and @jasmussen.

Thank you for the ping. Yes, for one, any item that has a pinned button in the toolbar must have an item in the more menu. Otherwise you can unpin that item and not get it back. I believe this is currently something the developer must ensure, but it would be nice if the API handled it so that a pinned item is an extension of a menu item.

And also, the pinned items do not need to have anything to do with sidebars, it could be sidebars, but it could be a popover, or a modal dialog, or hey, a link that opens a new tab. Think of it very much like a Chrome or Firefox extension in that way.

@gziolo Are you saying it's currently possible to have a pinned icon that does something other than open a sidebar? I've been struggling to figure out how to do that in the shipping version of Gutenberg. I think it would be an awesome addition though and makes sense to ensure it's accommodated here.

@chrisvanpatten - unfortunately it isn't implemented for modals and menu items that call arbitrary actions. Priorities change too often so I never got back to those APIs. However, technically speaking it should be straightforward to replicate.

As @jasmussen mentioned, we need to fix the issue for the sidebar first - #14457. In that issue, there is a proposal where the plugin sidebar would have a corresponding menu item attached by default.

gziolo · 2020-02-21T14:12:59Z

packages/plugins/src/components/plugin-sidebar/index.js

+import SidebarHeader from './sidebar-header';
+
+function PluginSidebarPinnedItems( { scope, ...props } ) {
+	return <Fill name={ `PluginPinnedAreas/${ scope }` } { ...props } />;


It made me think that maybe we should support scopes natively in the SlotFill? What do you think? If I recall properly there are more places like that where we have to play with the name of the SlotFill pair.

gziolo · 2020-02-21T14:24:04Z

packages/plugins/src/components/plugin-sidebar/index.js

+				isActive: getSingleActiveArea( scope ) === sidebarName,
+				isPinned: isMultipleActiveAreaActive( scope, sidebarName ),


It's a bit confusing to see the names of selectors so loosely related to the local constants used. Can you explain how multiple active areas relate to pinned items? I might be missing something because the notion of active areas is brand new.

For reference, this is how it is implemented in edit-post package:

gutenberg/packages/edit-post/src/components/sidebar/plugin-sidebar/index.js

Lines 149 to 150 in 8bba7e1

isActive: getActiveGeneralSidebarName() === sidebarName,

isPinned: isPluginItemPinned( sidebarName ),

Aside: to be clear, I don't like getActiveGeneralSidebarName at all :)

gziolo

Thank you for working on this PR. It's quite challenging to build a good abstraction that uses names that are flexible enough to cover all uses cases that we deal with including native mobile. In general, this proposal is very good. I have some issues with the names proposed but I'm biased because I know how edit-post was implemented. The fact that we never implemented plugins for modals and menu items with actions makes it harder to process. I would appreciate help from someone who can judge better what's the best approach to take taking into account feedback shared.

youknowriad

Was talking to @jorgefilipecosta about the need for a @wordpress/admin-screen package to handle the common things across WP screens (Skeleton, Fullscreen, Plugins) instead of adding these APIs to the plugins package.

Also, I'd like to see how this would apply to edit-post as it's a more concrete use-case and the refactor wouuld give us a clear idea about the needed APIs

jorgefilipecosta · 2020-03-09T11:42:41Z

Closed in favor #20698.

jorgefilipecosta added [Type] Enhancement A suggestion for improvement. [Feature] Plugins API Extending the Gutenberg project with plugins via the Plugins API labels Feb 20, 2020

jorgefilipecosta requested review from adamsilverstein, gziolo and TimothyBJacobs as code owners February 20, 2020 15:55

jorgefilipecosta force-pushed the add/sidebar-functionality-to-plugins-package-and-use-in-side-edit branch from ceaddbd to 09a1004 Compare February 20, 2020 18:35

jorgefilipecosta force-pushed the add/sidebar-functionality-to-plugins-package-and-use-in-side-edit branch from 09a1004 to 02c8dd0 Compare February 20, 2020 19:11

Add: Plugin Sidebar management functionality to plugins package. Use …

0bcec0e

…the functionality on edit-site.

jorgefilipecosta force-pushed the add/sidebar-functionality-to-plugins-package-and-use-in-side-edit branch from 02c8dd0 to 0bcec0e Compare February 20, 2020 19:35

gziolo reviewed Feb 21, 2020

View reviewed changes

youknowriad reviewed Mar 5, 2020

View reviewed changes

chrisvanpatten mentioned this pull request Mar 5, 2020

Exploration: moving options #19128

Closed

jorgefilipecosta mentioned this pull request Mar 7, 2020

Add interface package with sidebar functionality #20698

Merged

jorgefilipecosta closed this Mar 9, 2020

aristath deleted the add/sidebar-functionality-to-plugins-package-and-use-in-side-edit branch November 10, 2020 14:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add: Plugin Sidebar management functionality to plugins package. #20336

Add: Plugin Sidebar management functionality to plugins package. #20336

jorgefilipecosta commented Feb 20, 2020

gziolo commented Feb 20, 2020

github-actions bot commented Feb 20, 2020 •

edited

Loading

jorgefilipecosta commented Feb 20, 2020 •

edited

Loading

gziolo commented Feb 21, 2020

gziolo Feb 21, 2020

jasmussen Feb 21, 2020

chrisvanpatten Feb 21, 2020

gziolo Feb 21, 2020

gziolo Feb 21, 2020

gziolo Feb 21, 2020

gziolo left a comment

youknowriad left a comment

jorgefilipecosta commented Mar 9, 2020

	{ isPinnable && (
	<PinnedPlugins>
	{ isPinned && (
	<Button
	icon={ icon }
	label={ title }
	onClick={ toggleSidebar }
	isPressed={ isActive }
	aria-expanded={ isActive }
	/>
	) }
	</PinnedPlugins>
	) }

		isActive: getSingleActiveArea( scope ) === sidebarName,
		isPinned: isMultipleActiveAreaActive( scope, sidebarName ),

	isActive: getActiveGeneralSidebarName() === sidebarName,
	isPinned: isPluginItemPinned( sidebarName ),

Add: Plugin Sidebar management functionality to plugins package. #20336

Add: Plugin Sidebar management functionality to plugins package. #20336

Conversation

jorgefilipecosta commented Feb 20, 2020

How has this been tested?

gziolo commented Feb 20, 2020

github-actions bot commented Feb 20, 2020 • edited Loading

jorgefilipecosta commented Feb 20, 2020 • edited Loading

gziolo commented Feb 21, 2020

gziolo Feb 21, 2020

Choose a reason for hiding this comment

jasmussen Feb 21, 2020

Choose a reason for hiding this comment

chrisvanpatten Feb 21, 2020

Choose a reason for hiding this comment

gziolo Feb 21, 2020

Choose a reason for hiding this comment

gziolo Feb 21, 2020

Choose a reason for hiding this comment

gziolo Feb 21, 2020

Choose a reason for hiding this comment

gziolo left a comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

jorgefilipecosta commented Mar 9, 2020

github-actions bot commented Feb 20, 2020 •

edited

Loading

jorgefilipecosta commented Feb 20, 2020 •

edited

Loading