Site Editor: Add template tab to sidebar #28633
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
david-szabo97
requested review from
mattwiebe,
jameskoster,
vindl,
Copons,
jeyip and
Addison-Stavlo
david-szabo97
commented
Feb 1, 2021
| */ | ||
| import { useSelect } from '@wordpress/data'; | ||
| import { Icon } from '@wordpress/components'; | ||
| import { page } from '@wordpress/icons'; |
There was a problem hiding this comment.
Do we have an icon representing a template?
|
Size Change: +1.41 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
jameskoster
requested changes
Feb 1, 2021
There was a problem hiding this comment.
Looking good for the most part!
The icon is slightly too small though – it should be 24x24.
Also, let's use the layout icon:
|
Fixed the size and changed the icon. This one looks different though 🤔 I used the layout |
|
Huh! Looks like that icon didn't get updated in the code. I'll handle that in a separate PR. I noticed the spacing and alignment doesn't match the icon/name/description on the block tab:
If we can tidy that up then I think we are good to go on the design side :) |
|
The funny thing is, I actually copied the styles of that layout. But there is a bug in that one that causes the icon to shrink if the description is long: Screen.Recording.2021-02-02.at.14.31.58.movHere is a recording with Screen.Recording.2021-02-02.at.14.35.48.mov |
|
Since the block card's styling is unrelated to this PR, I'll spin up a new PR for that one. |
david-szabo97
force-pushed
the
add/edit-site-sidebar-panel-template2
branch
from
ce12171 to
73a0f0c
Compare
|
Rebased to update the icon. |
jameskoster
approved these changes
Feb 2, 2021
TestingBehavior
Browsers
|
|
It seems we're getting closer to briding edit-post and edit-site. Opening the template in the post editor already shows a "template" tab on the sidebar. I believe at some point edit-site could become just a proxy to edit-post with a bunch of extra settings... |
david-szabo97
requested review from
ajitbohra,
nerrad and
ntwb
as code owners
david-szabo97
force-pushed
the
add/edit-site-sidebar-panel-template2
branch
from
a40c627 to
dcda8cd
Compare
Copons
reviewed
Feb 10, 2021
| import { STORE_NAME } from '../../../store/constants'; | ||
| import { SIDEBAR_BLOCK, SIDEBAR_TEMPLATE } from '../constants'; | ||
|
|
||
| const SettingsHeader = ( { sidebarName } ) => { |
There was a problem hiding this comment.
Nitpick: what about calling this SidebarHeader?
I was thrown off by the difference between component name and file name (sidebar/header).
In the same vein, it would also be fine to rename the folder to sidebar/settings-header (but imho it's less clear).
There was a problem hiding this comment.
I renamed the folder to settings-heade. edit-post is using sidebar/settings-header as well. SettingsSidebar is only used together with the default (settings) sidebar (it's not used with the global styles sidebar). That's why I'd keep the Settings in the name.
There was a problem hiding this comment.
I think it makes sense to align the naming with edit-post here.
Copons
approved these changes
Feb 10, 2021
vindl
reviewed
Feb 10, 2021
|
Seems like this PR might have broken an e2e test on master. Or at least, the failure started happening with this merge. |
Created a draft PR to see if that fixes the e2e tests. #28950 |
|
Looks like some changes were made in this PR #28835 that removed the custom component based toolbar control. This PR still included that check so probably that's why it's failing. |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add template tab to settings sidebar. Resolves #27470
How has this been tested?
Screenshots
Screen.Recording.2021-02-01.at.14.51.35.mov
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: