-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Details: Enable all non-interactive formats #68741
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
Details: Enable all non-interactive formats #68741
Conversation
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Can you please test if you can make the language (and directional) option to work? When I try to use the "Apply" button, the details block opens and closes and the option does not seem to save. Edit: its not only the apply button. If I click to select the language input field, it also closes or opens the details. |
|
@im3dabasia, do you mind rebasing on top of the latest trunk and resolving the merge conflict? |
720e25a to
ed64574
Compare
|
I’d like to confirm that the language option does not seem to be working as expected. It appears that when the Apply button is clicked, the settings are not applied, and the modal remains open. Language and direction option Screen.Recording.2025-01-29.at.9.00.29.PM.movAll other options Screen.Recording.2025-01-29.at.8.59.37.PM.movI’m looking into this and will try to find a fix. |
|
Hey @Mamaduka, After debugging for a while, I found that I believe the issue is with the onClick event added to the After adjusting the code slightly, I was able to apply both the language and direction options without any issues. This happens because events are likely delegated or bubbled to a common parent, possibly the language modal and the summary element, causing unintended behavior. Consider the following structure: <div> <!-- Common Parent -->
<div> <!-- Deep inside: Details & Summary Block -->
<details>
<summary>Click Me</summary>
<p>Some extra details...</p>
</details>
</div>
<div> <!-- Modal Wrapper -->
<div class="modal">Language Settings</div>
</div>
</div>The modal's actions, such as onClick, are bubbling up to the common parent. Since the open/close state of Code change which worked for meUsed onToggle in the details block along with e.target.open.diff --git a/packages/block-library/src/details/edit.js b/packages/block-library/src/details/edit.js
index ded12c15fc..693e0fb310 100644
--- a/packages/block-library/src/details/edit.js
+++ b/packages/block-library/src/details/edit.js
@@ -91,13 +91,12 @@ function DetailsEdit( { attributes, setAttributes } ) {
) }
/>
</InspectorControls>
- <details { ...innerBlocksProps } open={ isOpen }>
- <summary
- onClick={ ( event ) => {
- event.preventDefault();
- setIsOpen( ! isOpen );
- } }
- >
+ <details
+ { ...innerBlocksProps }
+ open={ isOpen }
+ onToggle={ ( event ) => setIsOpen( event.target.open ) }
+ >
+ <summary>
<RichText
identifier="summary"
aria-label={ __( 'Write summary' ) }ScreencastScreen.Recording.2025-01-29.at.11.06.01.PM.movLet me know your thoughts! |
|
@im3dabasia, what's the status on this PR? @shimotmk, did you get a chance to test this? |
e98d8ea to
3e98c71
Compare
|
I have made the code changes and updated the PR. I used onToggle on the details element rather than an onClick on the summary element. I explained my reasoning in my observations #68741 (comment). Please let me know what you think about this approach. I'm attaching a video reference to demonstrate that all formats now work correctly. Screen.Recording.2025-03-28.at.12.14.46.PM.mov |
Mamaduka
left a comment
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.
Thanks, @im3dabasia! Works like a charm ✅
Co-authored-by: im3dabasia <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: Mamaduka <[email protected]> Co-authored-by: shimotmk <[email protected]>
Co-authored-by: im3dabasia <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: Mamaduka <[email protected]> Co-authored-by: shimotmk <[email protected]>
Co-authored-by: im3dabasia <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: Mamaduka <[email protected]> Co-authored-by: shimotmk <[email protected]>
|
This PR was not originally planned to be backported to WP 6.8.2, but was cherry-picked to resolve a conflict when cherry-picking other releases. See #70557 (comment) for more details. |
Closes: #68738
What?
PR removes hardcoded allowlist for RichText formats in the Details blocks. Now, all non-interactive formats are allowed within these blocks.
This changes matches the behaviour of the button, navigation block
Why?
Allows consumers to register custom formats that can be used with details blocks.
Testing instructions
Screenshots or screencast