Add interface package with sidebar functionality #20698
Conversation
jorgefilipecosta
requested review from
ajitbohra,
chrisvanpatten,
mkaz and
youknowriad
as code owners
|
Size Change: +4.23 kB (0%) Total Size: 861 kB
ℹ️ View Unchanged
|
jorgefilipecosta
force-pushed
the
add/admin-screen-package-move-sidebar-functionality-admin-screen
branch
2 times, most recently
from
2ef0838
to
ee38298
Compare
jorgefilipecosta
force-pushed
the
add/admin-screen-package-move-sidebar-functionality-admin-screen
branch
4 times, most recently
from
924d9b5
to
e1ed846
Compare
|
|
jorgefilipecosta
force-pushed
the
add/admin-screen-package-move-sidebar-functionality-admin-screen
branch
2 times, most recently
from
46fa964
to
4343c53
Compare
|
@mtias we have different things we want to reuse across screens. Sidebar Plugins, EditorSkeleton (PageSkeleton), FullscreenMode What do you suggest as a name? |
|
I know it's already in use, but this would be the "editor" package to me (as opposed to the block editor). |
|
Maybe editor-screen would be a good name? |
|
Can you list all the "editor" related packages we'd have? |
Hi @mtias, |
|
What are the conceptual differences among |
Hi @mtias,
|
|
|
|
Do you think these components/tools (Skeleton, Sidebar, Fullscreen) are things specific to editor screens or any Admin screen? Personally, I'm thinking the latter. |
|
Whatever the final decision is, we should improve the documentation for all mentioned packages and provide a high-level overview of how they all fit together. It's always been an issue for all people that start with Gutenberg development. |
Good point, I agree these UI artifacts may be the basis to a screen in WordPress that may not necessarily contain an editor, e.g: I may imagine this artifact may contain a system to manage tickets, and the sidebars show information about the person that submitted the ticket. What do you think @mtias? |
jorgefilipecosta
force-pushed
the
add/admin-screen-package-move-sidebar-functionality-admin-screen
branch
from
cc9ae9b
to
af54fbb
Compare
Squashed commits: [55e02fd47e] Update packages/edit-post/src/components/layout/index.js Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> [7aa42cba13] Update packages/interface/src/components/plugin-complementary-area-header/index.js Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> [13abfa99d2] Update packages/interface/package.json Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> [61b5b1086e] Update packages/interface/package.json Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> [b5f425096c] Update packages/interface/src/components/pinned-items/index.js Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> [2e5bb80774] Update packages/edit-post/src/components/header/index.js Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> [40e1bb4c4e] Add interface package with sidebar functionality
Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]>
jorgefilipecosta
force-pushed
the
add/admin-screen-package-move-sidebar-functionality-admin-screen
branch
from
af54fbb
to
78d8de7
Compare
jorgefilipecosta
deleted the
add/admin-screen-package-move-sidebar-functionality-admin-screen
branch
| function Sidebar() { | ||
| return ( | ||
| <div className="edit-site-sidebar"> |
There was a problem hiding this comment.
@jorgefilipecosta want to check why we removed this, because I'm adding it back at https://github.com/WordPress/gutenberg/pull/20530/files?file-filters%5B%5D=.js#diff-3924a41ad88da62082971d8c0aa8b20fL14
The style.css is still targeting that class. I've only noticed after the changes made to the hooks (color, line-height), because some styles were leaking to the edit-site sidebar.
| @@ -62,7 +62,7 @@ export default function Header() { | |||
| </div> | |||
| <div className="edit-site-header__actions"> | |||
| <SaveButton /> | |||
| <MoreMenu /> | |||
There was a problem hiding this comment.
Was this removal intentional or did it just end up there during the rebase?
There was a problem hiding this comment.
Probably a rebase issue.
There was a problem hiding this comment.
Probably a rebase issue.
Ok, I'll spin up a quick PR to bring it back. Update: merged in #21288
|
@vindl @jorgefilipecosta Should we move EditorSkeleton and Fullscreen to the new package? |
|
Just wanted to highlight my comments that weren't addressed:
I don't remember if I mentioned it but users will lost their saved preferences for pinned items because the migration path is missing. It was persisted in the local storage in the |
As for the Fullscreen - yes, I think so and I can look into this soon. As for the EditorSkeleton, I'd have to check its code a bit more before I can say for sure. |
Brings back the Site Editor more menu that was likely lost as a result of a rebase in a #20698.
@youknowriad I started the PRs for moving those in: #21334 and #21335. |
|
Note: I added the |
Addressed in #21417. |
Hi @gziolo, I will soon submit a set of unit tests the functionally of this package. I just want to understand the shape the package will follow as things can still change so we don't waste effort on tests that may need updates.
I think it was discussed before, personally I think losing the pinned items is not a big issue as users can easily update them, add the migration path may make our code more complex. But if you all think we should have some migration I can propose something. Any thoughts on this @gziolo, @youknowriad? |
| const { pinItem, unpinItem } = useDispatch( 'core/interface' ); | ||
| return ( | ||
| <> | ||
| { isPinned && ( |
There was a problem hiding this comment.
The fact that isPinned is now outside of the <PinnedItems /> fill created a regression as described in #28546 =.
Description
Supersedes: #20336
This PR starts an admin-screen package suggested by @youknowriad in #20336 (review).
This package for now just contains the sidebar functionality but other functionality will be added (e.g: fullscreen mode).
The sidebar functionality is renamed the ComplementaryArea as suggested by @gziolo.
This package, for now, is experimental should be private and is bundled with other packages.
Regarding the admin screen store:
We implement 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...
We implement 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.
How has this been tested?
I verified the edit-post sidebar still works as expected (namely auto-closing on mobile, keyboard shortcuts, plugin sidebar).
I verified the sidebar mechanism added on edit-site works as expected.