Size Change: +197 B (+0.01%)

Total Size: 1.96 MB

_{compressed-size-action}

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: jarekmorawski <jarekmorawski@git.wordpress.org>
Co-authored-by: jeffpaul <jeffpaul@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

I know these are private actions/selectors, but terminology is still specific to Block Comments. A more general solution, even if it's private for now, seems more suitable.

What I had in mind:

• A toggleBlockSpotlight action, which is the same as toggleBlockHighlight, but handles block spotlight. I think the state reducer can be the same.
• A hasBlockSpotlight selector or whatever you want to call it, which toggles the existing CSS classes.

@Mamaduka Thanks for the review!

Personally, I avoided toggleBlockSpotlight and hasBlockSpotlight because they seemed like they could be confused with spotlight mode. I don't have a strong opinion on the terminology, but what do you think?

Flaky tests detected in f84928b.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18337000273
📝 Reported issues:

• [Flaky Test] should duplicate and activate #71988 in /test/e2e/specs/site-editor/template-activate.spec.js

Looks great at a glance. I'm now subscribed to this PR and hope to find time for a more thorough review (e.g. how does it look with the pinned sidebar closed), but it looks on a decidedly good track. Kudos.

Personally, I avoided toggleBlockSpotlight and hasBlockSpotlight because they seemed like they could be confused with spotlight mode.

This action/selector "puts a spotlight" on a block, which is why I suggested those general names. There might be other features that could benefit from using this method in the future.

It's not a blocker; we can rename them if needed.

We'll have to modify the selectors' return value, but was there any downside to updating the isFocusMode condition below, vs useBlockProps?

We'll have to modify the selectors' return value, but was there any downside to updating the isFocusMode condition below, vs useBlockProps?

@Mamaduka Sorry, maybe I don't understand the question, but do you mean we will also be incorporating this new API into Spotlight mode?

how does it look with the pinned sidebar closed

@jasmussen It will look like this:

@t-hamano, I was suggesting reusing the existing condition in the linked file and the CSS styles that are already there. Just a random thought, it's not a blocker.

This looks good to me! I wonder if we should also fade the unrelated comments, just like we fade the blocks in the canvas.

A few other thoughts. They're unrelated to this PR, so please let me know if I should take them elsewhere.

1. I don't think we need to expand the thread when a comment is selected. It'd get out of hand for longer conversations.
2. Could we replace the gray background with white for a cleaner look and better readability?
3. We don't need the user's name and avatar at the end of the thread. It's perfectly fine to display just the input field and their details after they publish the comment.

@jarekmorawski Thanks for the feedback!

As for the proposed design change, it might be better to open a new issue. This PR is just to highlight the block, so we won't make any changes to the comment itself.

Some useful links:

• Tracking Issue: Notes iteration for WordPress 6.9 #66377
• Figma design: https://www.figma.com/design/CSlNU8T0EQ0dhLVIppNwFl/Annotations-Comments-Collaborative-Editing?node-id=233-2752&p=f&t=SDaOd3vb0y609bMC-0

A meta thought: @adamsilverstein has been exploring collapsed versions of the comment threads when there isn't room to show them perfectly aligned with the block itself. Not a blocker for this PR, probably not for release either, but it will very probably benefit the particular pattern used in the example here.

I wonder if we should also fade the unrelated comments, just like we fade the blocks in the canvas.

Nice instincts. Important to balance that also with the gray color in the inactive comments, as well as the contexts for legibility. It may be fine.

I wonder if we should also fade the unrelated comments, just like we fade the blocks in the canvas.

Nice instincts. Important to balance that also with the gray color in the inactive comments, as well as the contexts for legibility. It may be fine.

I see, let's try this in a separate PR.

Just noting that this PR had an impact on "block selection" metric. Any idea what caused this?

Thanks for pointing that out, @youknowriad!

Yes, that seems like a big jump - https://www.codevitals.run/project/gutenberg/focus.

Is it because hasBlockSpotlight has many actions to unblock the spotlight?

Can we somehow measure this metric locally or before the merge?

@t-hamano, the performance test CI job outputs the results, but it's not as reliable as CodeVitals - https://github.com/WordPress/gutenberg/actions/runs/18337000307.

You can also run tests locally for debugging - npm run test:performance -- post-editor.spec.js. Do something like test.describe.only( 'Selecting blocks' to run focused tests.

@t-hamano, push an experiment for an alternative method - #72194, let's see if it has any effect on block selection performance.