-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Flash editable block outlines instead of always showing them #58159
Conversation
Size Change: +125 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
Flaky tests detected in 29fd351. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7648370740
|
This is an improvement over what's in trunk so LGTM once we're happy with the technical side of things. We can keep iterating until we find the right balance between visibility and it being a distraction. |
Isn't it supposed to be a distraction? Almost feels like the treatment could be heavier / more obvious. One observation (probably doesn't need to hold up this effort): the effects when clicking non-editable content seem contradictory. The Snackbar suggests editing the template, while the outlines imply editing the content. This feels a bit unsettling, like being pulled in opposite directions. |
This is an improvement, thanks for sweating the details. That said, it wasn't entirely what I expected, I think we can finetune it a little bit. Specifically, I wouldn't expect it to be present from the start — that especially makes those borders appear like content, rather than editor guides. Secondly, the fading and time before fading out feels very slow, I think that actually detracts from the emphasis they are meant to provide. The behavior I was expecting was that you click to edit a page, and if you never hover anything other than the Post Content block, you won't see them. But as soon as you hover the header, or the footer, or something other than the post content, those outlines fade in fast, and very shortly after, fade out. They might even pulse, if need be, to provide all the more emphasis. In them appearing fast, pulsing, then disappearing, and only when hovering other-than-post-content, there would be an immediate connection to the behavior: no, not here, down there is where you edit. Would love @richtabor thoughts too. Separately, I still find it a bit confusing that the post content block is full-width, when the title block is not. Is this a result of the width of the elements in the DOM, or an intentional choice? |
In practise I fear this would mean you'd be seeing the outlines much more often. For empty pages in particular, the majority of the canvas will be 'template'.
This will be due to the template block structure. E.g. Title may be in a Group where Content is root level. |
We're highlighting the border box (what's clickable). We could probably highlight the content box (what an inner block could expand to fill) with some work—we'd have to stop using pseudo |
Updates the editable block outlines that appear within content-locked containers (added in #57901) to appear and then fade out after 3s when: - The page loads; or - The user clicks on the content-locked container. This is done via a private useFlashEditableBlocks() hook attached to the container.
3c6a1d8
to
d7671f0
Compare
I finished the remaining TODOs, rebased this, and reduced the animation delay to 1s. |
I think let's get this in as it's a big improvement on what's already in |
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.
Code looks great, and behaviour-wise this feels terrific to me! 1 second fade feels like a nice amount of time, and it feels like a good level of responsiveness while being unobtrusive to me when I go to click on the template.
✅ Works with partially synced patterns
✅ Works with content only locked patterns
✅ Works with pages edited in the site editor
✅ Works with posts in the post editor in template preview mode
2024-01-31.14.14.44.mp4
LGTM! ✨
block.classList.remove( 'has-editable-outline' ); | ||
// Force reflow to trigger the animation. | ||
// eslint-disable-next-line no-unused-expressions | ||
block.offsetWidth; | ||
block.classList.add( 'has-editable-outline' ); |
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.
Oh, clever!
if ( renderingMode !== 'template-locked' ) { | ||
return; | ||
} |
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.
Just double-checking that I'm following correctly: the renderingMode
is removed from the checks in the click and double click handler because we're moving the check up to where the component is being output instead of making it internal to the component?
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.
Yeah you have it right.
It's not necessary for this PR at all—leftover code from some explorations I was doing that went nowhere.
But I kept these changes as I think it's much nicer to simply not mount the component at all if it's not needed rather than unnecessarily mount the component and attach all these unnecessary event listeners that run unnecessary code on every click.
The outline behavior is working pretty well for me. But I still find them plus the Snackbar too much. The outlines disappear before I have time to read the Snackbar message, so each affordance effectively detracts from the other. Is it worth trying a version without the Snackbar? |
Yeah I'm not fond of the snackbar notifications now that we have the outlines. The only thing that gives me pause re. removing them is that it's currently the most convenient way of accessing the template. I think we'd need to make the Edit template button (currently buried underneath the Template menu in the document settings) more prominent. Was it you @jameskoster that had a mockup of putting that button in the View menu alongside Desktop, Tablet, Mobile? I like that. What do you think @SaxonF? At any rate, let's keep firing away on all that in follow-up PRs 😀 |
Yeah there are so many options to explore there, let's continue refining in subsequent issues/prs. |
What?
Iterates on #57901.
Updates the editable block outlines that appear within content-locked containers (added in #57901) to appear and then fade out after 1s when:
Why?
See discussion in #57901 (comment). We want to reduce the prominence of these outlines as they are currently too distracting from the writing experience.
How?
A
.has-editable-block-outline
class is added to blocks that are editable but within a locked container. This class shows the outline and then fades it away after 1s.Then, a private
useFlashEditableBlocks()
hook is attached to relevant containers: template-locked pages, synced patterns, and patterns withtemplateLock = 'contentOnly'
. The hook listens for aclick
on a disabled element and replays the animation when this happens.Testing Instructions
The outlines should should appear when editing a content only locked pattern, a pattern with instance overrides, and a page in the site editor.
Content only locked patterns:
contentOnly
lock. Here's an example of one.Pattern with instance overrides:
Page in the site editor:
Screenshots or screencast
Kapture.2024-01-31.at.10.54.31.mp4