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.
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
Site Editor: Add ability to focus on editing a page's content vs the page's template #50857
Site Editor: Add ability to focus on editing a page's content vs the page's template #50857
Changes from 41 commits
5f3c1b2
bcf6949
6ab13d7
6426fca
bd2f478
f00dc25
c7e128c
2a9b9e5
692cd76
8162afe
52c4df0
3d437a4
25f3e58
f1e1a11
6bf1ddd
727ecd2
6bd3ab8
12e8ad4
4489da2
fda46cd
0443db4
8c1c4a6
bed91ec
23837de
3cd4a17
599a4d1
5d52359
055b7f1
79c59bf
726c6cc
89212cd
bdee82a
0395bf0
67cf0db
50748c2
a468d55
261d076
64d0e43
c860258
c22368a
32b5413
9eba206
245a0bb
97e8f23
46c4074
cd90a63
7e64fa5
fa8133b
f3fc2ca
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The black dot is a collapsed border style, and appears on all page blocks that are
contentOnly
, e.g., feature image block as well.There's a permanent border on the following element (I've added
border:0
inline to remove it via the console)This element is rendered by
<BlockContextualToolbar />
(block-contextual-toolbar.js). The offending CSS rule for.block-editor-block-contextual-toolbar
is in style.scss.The element is always rendered, even when the slots are empty.
I'm not sure what the best approach is. Checking via
useSlotFills()
whether the slot is, indeed filled? Would we have to loop through all groups.js?I'm also guessing the value of
blockEditingMode
inmight not be useful to since the toolbar can show for
defaultand
contentOnly. In this case the site title block has a value of
contentOnly`, but the slots are not filled.Just dumping my brain here in case it helps... :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for digging into this. I'm also not sure how best to fix it. It's a shame we can't use the
:empty
pseudo selector in CSS because the toolbar technically isn't empty (there's a<div />
). Checking if there's fills viauseSlotFills()
is probably the best bet like you say.At any rate I'm going to leave fix this for a follow-up PR. (It's noted in the PR description.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we do something like :has( > div:empty:only-child)? Fine to address as a follow-up though 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:has
doesn't work in Firefox 😭 I'll pick your brain about this next week.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still able to edit/interact with block toolbars in the header when in content editing mode, though the toolbars no longer show the block type:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I don't see that... what steps did you take?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can too - site title and paragraphs in the header and footer seem to be editable. Not sure I did anything special.
I also noticed that when switching from editing the template back to editing the page content, the block you have selected might be part of the template, but it remains selected and has a toolbar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed the same. This PR is otherwise testing quite nicely for me, though!
Here's a screengrab in case it helps:
2023-05-25.16.22.38.mp4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have the latest changes pulled? There was a bug where you could select a locked block by double clicking or dragging that should be fixed in 3cd4a17.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, just tried re-pulling, and my environment says it's up to date, and I re-ran
npm run dev
. In the screengrab, I'm single-clicking the Site Title block in the header, but I can also reproduce by clicking the Site Title or Paragraph blocks in the footer, too.Very weird — I can see the
pointer-events: none
rule should be applying, but I was still able to select the block 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have latest changes too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you using TT3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, looks like it only happens in Chrome. I'll investigate tomorrow.