-
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
Disable the ability select text in the useBlockPreview
hook
#47331
Conversation
Size Change: +3.85 kB (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in 932e8e2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3986580713
|
Thanks for the PR, I have two questions here:
|
I also was not able to reproduce the issue initially by following the instructions here. (I mean without the fix) |
This seems to have to do with the Post Excerpt block, when you click in the |
Thanks for the advice.
I was also able to reproduce it. The problem occurs when I click on the 'Add "read more" link text' area in this block. In addition to this, I have found a very strange point. Changing the diff --git a/packages/block-library/src/post-excerpt/style.scss b/packages/block-library/src/post-excerpt/style.scss
index e7b3e18491..0cb071e25d 100644
--- a/packages/block-library/src/post-excerpt/style.scss
+++ b/packages/block-library/src/post-excerpt/style.scss
@@ -1,3 +1,3 @@
.wp-block-post-excerpt__more-link {
- display: inline-block;
+ display: block;
} Of course, this problem should not be fixed by this change, but one wonders why it would be fixed by this change 🤔 |
I found about the Having said that Riad mentions a different issue:
It doesn't need to be part of this PR of course, but Post Template block uses the same blocks for the post items, that means they share the same |
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.
Personally I think it's okay to land this for now, but we need to investigate if we can find a better solution..
Thanks!
packages/block-editor/CHANGELOG.md
Outdated
@@ -10,6 +10,7 @@ | |||
- Move component styles needed for iframes to content styles ([#47103](https://github.com/WordPress/gutenberg/pull/47103)). | |||
- Block Inserter: Correctly apply style to the default inserter ([#47166](https://github.com/WordPress/gutenberg/pull/47166)). | |||
- List View: Fix crash when the first template-parts is deleted width del key ([#47227](https://github.com/WordPress/gutenberg/pull/47227)). | |||
- Block Preview: Try to fix multiple rendering of controls ([#47331](https://github.com/WordPress/gutenberg/pull/47331)). |
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.
Probably change the message here to explain that you disabled the ability select text in the useBlockPreview
hook.
useBlockPreview
hook
Thanks for the review. I would like to merge this PR, but how about leaving #47293 open to explore the root cause? |
useBlockPreview relies on useDisabled to disable interactions within it. If I'm not wrong useDisabled applies the "inert" attribute which also is supposed to disable selection no? so why does it not work by default here? |
From what I've researched, diff --git a/packages/compose/src/hooks/use-disabled/index.ts b/packages/compose/src/hooks/use-disabled/index.ts
index 7a384cbcc5..7d5cf135aa 100644
--- a/packages/compose/src/hooks/use-disabled/index.ts
+++ b/packages/compose/src/hooks/use-disabled/index.ts
@@ -34,16 +34,21 @@ export default function useDisabled( {
isDisabled: isDisabledProp = false,
} = {} ) {
return useRefEffect(
- ( node ) => {
+ ( node: Node ) => {
if ( isDisabledProp ) {
return;
}
+ const defaultView = node?.ownerDocument?.defaultView;
+ if ( ! defaultView ) {
+ return;
+ }
+
/** A variable keeping track of the previous updates in order to restore them. */
const updates: Function[] = [];
const disable = () => {
node.childNodes.forEach( ( child ) => {
- if ( ! ( child instanceof HTMLElement ) ) {
+ if ( ! ( child instanceof defaultView.HTMLElement ) ) {
return;
} |
Not sure I follow, is this because of the polyfill that is not loaded in the iframe or something else? Regardless, if feels we shouldn't hack something in this PR instead we should fix the root issue which is why useDisabled is not working inside the iframe |
I would like to freeze this PR and try to find the root cause of this problem. If we can get the However, as we investigated in #47190, it may not work correctly in FireFox, so we will continue to investigate this issue as well. |
I noticed that the original issue only occurs for me when I select the read more link in a post after the first within the Post Template block. This PR tests well in terms of fixing the issue but I agree that it would be better to ensure
The |
Fixes: #47293
What?
This PR fixes a bug that caused multiple rendering of controls when a block was selected in the site editor. This problem, as reported in #47293, seems to occur sometimes when selecting a block in a post template.
Why?
I have confirmed that this problem doesn't occur when the Gutenberg plugin is disabled. I can't explain the root cause, but I noticed the following different style attributes in the rendering of the post template.
In other words, I thought that the
user-select:none;
style should always be applied to theli.wp-block-post block-editor-block-preview__live-content
element in order to achieve the expected behavior.How?
Added
user-select: none;
in the.block-editor-block-preview__live-content
selector.As an alternative approach, I have confirmed that the following changes will also solve the problem.
Testing Instructions