-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[Editor]: Select blocks in outline
list
#48118
Conversation
Size Change: +16 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in 66b66e5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4202719754
|
onSelect={ onSelect } | ||
onSelect={ () => { | ||
selectBlock( item.clientId ); | ||
onSelect?.(); |
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.
This this used to work because of the link to the anchor. I wonder if we can trigger a URL change in the iframe...
Thank you for the PR! I'm wondering if this link should be a button rather than an anchor to begin with. Because in the non-iframe editor, the selected block is not centered, perhaps because the anchor link takes precedence: 2dd93dc66c3ddd38538325d8983ff639.mp4As follows, I would expect the selected block to be centered, iframe editor or not: 03c95edc1f8cc9fe59c3bf6a03f2bca5.mp4Anchor links were initially implemented in #10815 with the expectation that the content would be in the same document. However, now that we cannot guarantee that the content exists in the same document, we might be able to revert to buttons. |
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.
Looks good. There's a potential alternative solution with the target
attribute which could open the link in a specific iframe (editor-canvas), but it doesn't work well out of the box because there's no initial URL, so it refreshes the page. So I think this is the best solution.
Should the href
links be removed and be converted into buttons? Fine either way for me.
d5a672a
to
66b66e5
Compare
@t-hamano let's discuss and do this in a follow up, as the current change fixes the iframed case and matches the non iframed one. What do you think? |
Sounds good, let's do that 👍 However, what about backporting this PR to WordPress 6.2? |
This needs no back port since the editor will not be iframed in core. |
66b66e5
to
a293977
Compare
What?
Part of: #47624
The heading links in the post outline no longer work. This is a problem when the post editor is iframed. This PR fixes that by also selecting the block.
The main issue has a few things to resolve regarding the Post Title. Right now it's not shown for iframed editor and the link is not working in non iframed editor.
Testing Instructions
Before
before.mp4
After
after.mp4