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: adamsilverstein <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: jeffpaul <[email protected]>
Co-authored-by: stokesman <[email protected]>
Co-authored-by: karthick-murugan <[email protected]>
Co-authored-by: kittenkamala <[email protected]>
Co-authored-by: youknowriad <[email protected]>

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

Size Change: +289 B (+0.01%)

Total Size: 2.38 MB

_{compressed-size-action}

Flaky tests detected in 759ac66.
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/18967676712
📝 Reported issues:

• [Flaky Test] ensures List View global shortcut works properly #51533 in /test/e2e/specs/editor/various/list-view.spec.js
• [Flaky Test] should allow user to add and remove multiple local font files #62862 in /test/e2e/specs/site-editor/font-library.spec.js

This approach more closely mimics Google Docs where lengthy comments scroll the doc content above the fold. I had originally thought that wouldn't look/work well in the block editor but that After video seems pretty fine to me.

Two questions @adamsilverstein is what happens if you have a drawer active below the post content (e.g. Yoast) as well as what happens if you scroll down past the post content and then click on a comment (will it scroll you back up to where the related block begins such that its back in frame or will the block and post content area still be above the fold)?

This approach more closely mimics Google Docs where lengthy comments scroll the doc content above the fold.

Exactly!

Two questions what happens if you have a drawer active below the post content (e.g. Yoast)

the code only sets the minimum height (min-height) for the editor frame body, so if an active drawer needs more space that will not be affected by this at all.

as well as what happens if you scroll down past the post content and then click on a comment (will it scroll you back up to where the related block begins such that its back in frame or will the block and post content area still be above the fold)?

I'm not changing that behavior - when you click on a comment it will scroll to the related block and pin alongside it. threads above will be pushed away above. New with this PR, to reach the bottom, you would just scroll down (in trunk, you can't reach the bottom, its cut off). This part might not be working perfectly in the PR yet, I'm going to work on that more.

This part might not be working perfectly in the PR yet, I'm going to work on that more.

I was able to correct the glitches and simplify the calculation in e616b5e. Previously I wasn't taking into account the editor scroll position in the calculations.

This resolves all the edge cases, give it a test in Playground.

Looking at the codebase, I think we could expose a private API from the editor to set the min-height, this would make the code more reactive. What do you think?

I took a pass at converting the code to use a private API instead of manipulating the DOM directly in 2e746ba and c854e93. We still need to include the scroll position in the calculation, but we can do that right where its available which is cleaner and more reactive.

Oops, those last commits were off, re-working shortly.

Ok, the reactive implementation is restored.

Pushed some refactorings:

• Replaced custom context with private store action/selector.
• Updated calculatedMinHeight memo to use localRef for getting scrollTop, in a more React-way manner.

Pushed some refactorings:

• Replaced custom context with private store action/selector.
• Updated calculatedMinHeight memo to use localRef for getting scrollTop, in a more React-way manner.

Thanks for the refactoring @Mamaduka - nice improvements! I tested this again and everything works as expected.

I was only able to trigger the wrong height in a few edge cases I couldn't reproduced and in every case all notes were fully visible (slightly too much space was added).

I briefly tested with playground and found that when adding notes to a block before or after one with a long note there was some behavior that surprised me. The worst was adding a note to a block after one with the long note, where the note was only accessible in the non-floating sidebar. Here’s a recording:

This seems like it should work quite well though. Good work!

@stokesman Thanks for testing! I was not able to reproduce the behavior you saw and I'm suspicious the playground build is out of date. Here is my screen recording for the record, maybe I missed a step:

There was a conflict while trying to cherry-pick the commit to the wp/6.9 branch. Please resolve the conflict manually and create a PR to the wp/6.9 branch.

PRs to wp/6.9 are similar to PRs to trunk, but you should base your PR on the wp/6.9 branch instead of trunk.

# Checkout the wp/6.9 branch instead of trunk.
git checkout wp/6.9
# Create a new branch for your PR.
git checkout -b my-branch
# Cherry-pick the commit.
git cherry-pick 6ac15ece4cc31917ed0808dcf310ca2e1ec12719
# Check which files have conflicts.
git status
# Resolve the conflict...
# Add the resolved files to the staging area.
git status
git add .
git cherry-pick --continue
# Push the branch to the repository
git push origin my-branch
# Create a PR and set the base to the wp/6.9 branch.
# See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request.

@Mamaduka the backport bot failed so worked on a manual backport for the 6.9 branch, can you please review - should be the same changes as this PR: #72886

Backported in #72886