Conversation
sethrubenstein
requested review from
ajitbohra,
fabiankaegy,
juanmaguitar and
spacedmonkey
as code owners
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @Dionnie. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
|
Just occurred to me, this will need block icons for all three blocks |
luisherranz
left a comment
luisherranz
left a comment
There was a problem hiding this comment.
This is awesome, Seth 🙂👏
Lastly, other plugins and consumers of this block can open/close or close all dialogs utilizing the Interactivity API, like so:
store('core/dialog').state.dialogs.[{the dialog's elm id}].isOpen = trueor of course identifying all dialogs on page viastore('core/dialog').state.dialogs.
We can combine private and public stores to make this block extensible without exposing all the internal details of the store publicly. It would be the first one from Core, but I think it would be good to do it so we can start exploring extensibility patterns. We can check it a bit later when the new implementation has advanced a bit more.
Other than that, I've only taken a preliminary look at the Interactivity API part, and in general, it's quite good, although here are a few small suggestions.
|
I'll be making a few more updates by end of the week, mostly around animations and your suggestions @luisherranz. |
@luisherranz on this topic, interesting, I would just do like this? // 1st party
store('core/dialog', {
state:{... internal derivced state funcs },
actions: {... internal funcs },
callbacks: {... internal callbacks }
}, {lock:true});
// Accessible by 3rd parties
const { state } = store('core/dialog', {
actions:{
open(id){
state.dialogs[id].isOpen = true;
}
}
}, {lock:false}) |
|
You need to use a different namespace. So it would be more like this: const { state, actions } = store( 'core/dialog/private', {
state: {
// internal derived state funcs
},
actions: {
// internal funcs
},
callbacks: {
// internal callbacks
}
}, {
lock: true
} );
store( 'core/dialog', {
// We add here the state that we want to be public.
state: {
// We can use getters for the state that we want to be read-only.
get dialog() {
return state.dialog;
},
// We can use setters for the state that we want to be modified.
get isOpen() {
return state.dialog.isOpen;
},
set isOpen( value ) {
state.dialog.isOpen = value;
},
},
// We add here the actions that we want to be public.
actions: {
open( id ) {
actions.open( id );
}
}
} );I've given the example of how to modify the private state if necessary, but for the most part, we're going to want the state to be read-only and to be modified through actions on public stores. EDIT: Oh, to return objects like function createReadOnlyProxy( obj ) {
return new Proxy( obj, {
get( target, prop ) {
const value = target[ prop ];
if ( typeof value === 'object' && value !== null ) {
return createReadOnlyProxy( value );
}
return value;
},
set() {
return false;
},
deleteProperty() {
return false;
}
});
}
store( 'core/dialog', {
state: {
get dialog() {
return createReadOnlyProxy( state.dialog );
}, |
|
Oh one other thing, I think I'm going to abstract out the dialog close button into it's own block so it can be more easily changed by 3rd parties. |
|
Actually, on further thought, I think I'm going to try to adopt |
|
Some updates from me:
|
There was a problem hiding this comment.
One thing I wasn't sure about (although I've found it critical to a good experience in the editor) is introducing a custom redux store for a block. I don't believe I've seen a core block do this before. Any thoughts, concerns against this pattern emerging?
There was a problem hiding this comment.
Going to replace this soon, just familiarizing myself with some recent updates to block attributes around role:local which will allow me to use a core method instead of a new redux store to manage this.
There was a problem hiding this comment.
Based on some feedback from @t-hamano in core/tabs on feasibility of maintaining these custom style engine implementations long term I'm rethinking how to handle the backdrop for dialogs. Once I wrap up this tabs refactor I'm going to come back in and create a new core/dialog-backdrop block that actually wraps the core/dialog-element block. From there background, alignment, justification, etc will make more sense and fall in line with core block use of style supports.
sethrubenstein
force-pushed
the
core/dialog
branch
from
5a7506d to
c4540ab
Compare
|
This is really cool to see, nice work so far. There are some design details I think we ought to tighten up before being able to merge:
|
All of these are great points that I'll take into account and address when I pick back up work on this on Monday. |
sethrubenstein
force-pushed
the
core/dialog
branch
from
add6991 to
252e3d6
Compare
|
Okay, I think... this is ready for review @annezazu . I (and Opus 4.5 under my direction per https://make.wordpress.org/ai/2026/02/01/ai-guidelines-for-wordpress/) made a number of updates to Per @jameskoster I also took care of these items:
|
|
I'm considering the image sub-block. It could potentially be interactive with a lightbox now that I think of it. I'll reevaluate some of the sub-block behaviors and clean up any potential interactive elements within the trigger with tag processor tomorrow. |
…t when selecting the block either in list view or other means.
…ade it impossible to remove dialog element. Added click handler for backdrop close click in editor.
…name presents as Dialog Wrapper. `core/dialog-element`, now just named Dialog.
…API server side instead of a purely server side block. Removes custom style engine implements. This follows the pattern from my work in core/tabs.
…alog wrapper directly.
…Icons_Registry for server side close icon in dialog. Notes and style cleanup.
…rovides the glue between editor and client side interactivity and the animation in style.scss. This also makes it possible for 3rd parties to change the animation, which pivotaly includes the duration. I agree that including a ui for the editor complicates this but this will allow devs to tap into the block more easily.
…jasmussen. Next up, tackling the accessibility issues around dialog trigger...
sethrubenstein
force-pushed
the
core/dialog
branch
from
7d1e7b4 to
64b419f
Compare
…ink. Update documentation and improve code formatting for better readability. Improved accessibility in frontend.
|
@jasmussen, I believe I've addressed all your review concerns. @joedolson, I also conducted a minimal accessibility audit and made some improvements. I would appreciate any feedback you can provide as well. I removed the "enable deep link" language and changed my original approach of registering a query variable. Instead, I implemented a client-side check for an ID hash in the URL that matches a dialog in state, allowing it to open. This method is cleaner and easier. Additionally, I have a solution in place for the trigger that wraps p/h/img blocks in a div with the role of a button. However, if the user only has a core buttons block inside, then we should only use that block for the trigger directives, avoiding nested button > button configurations. Aside from that, I'm not sure what else needs to be done here, if anything. I anticipate some pushback regarding the inclusion of a custom color attribute and controls for the backdrop. I felt that creating another block like |
|
Thanks for working on this, much appreciated. Just a quick GIF to visualize my quick run-through:
I have more feedback, but I think that should possibly/probably be addressed separately, because with the PR being long and complicated, it's probably best to code-review this and land it as the first iteration. Which is also to say, kudos, nice work! Some of those details to ponder:
Notably the navigation overlay now supports full customization through a template part, so themes can provide the design for the overlay. There are some mockups for overlay customization here, would be nice to share code there. |
|
Looking great! In terms of ability to be full width/wide width, is there any talk of sharing some of the implementation of how the navigation overlay template parts are being made? Apologies if it was covered. I did command-F for "wide, full, width, and 73084" and didn't find. #73084 |
|
👋
https://github.com/user-attachments/assets/fda5ff4a-e2ed-4340-a470-81538cbfc3e6
|
8 participants
What?
Closes #61297
#61297
This PR introduces 3 blocks from prc-block-library.
core/dialogcore/dialogis actually a collection of three blocks: 1.core/dialoga wrapper really for 2.core/dialog-trigger, and 3.core/dialog-element.core/dialog-triggerThis block is the "trigger" or "action" to open the dialog-element
<dialog/>block.core/dialog-elementThis is the primary block and is a representation of the
<dialog/>element in modal form. With added support for positioning center, top left, top center, top right, center left, center right, bottom left, center, and right and support for backdrop coloring it utilize core supports for box shadow and border quite well.Additionally, this block and it's iAPI store have been designed to be maximally extensible. We utilize this block throughout pewresearch.org, most complexly in RLS https://www.pewresearch.org/religious-landscape-study/age-distribution/18-29/?dialogId=dialog_prayer-frequency&activeChartId=6ac4c46314ff76e21be70e1f66fe1a19. This link shows off another feature built into
core/dialog, in this case a modified version of it's built in Deep Linking feature. Which allows you to open a dialog on page render so long as it's id is in the url with?dialogId. There is also an Auto Activation timer for opening a dialog immediately on page render based on a ms timer. Lastly, other plugins and consumers of this block can open/close or close all dialogs utilizing the Interactivity API, like so:store('core/dialog').state.dialogs.[{the dialog's elm id}].isOpen = trueor of course identifying all dialogs on page viastore('core/dialog').state.dialogs.Lastly, there is a block binding provided to associate a heading with the dialogLabel attribute for
core/dialog-element.This provides an easy and familiar interface to provide a standard paradigm ~ a heading at the top of a dialog while also allowing the user to set the label and remove the display block, the heading, from the dialog. In either case, if a heading is not present a dynamic heading is created for accessibility and hidden from view on the frontend.
Why?
Dialogs, or "modals" are a common UI pattern that many 3rd parties have attempted. This provides a base and a representation of an actual HTML element,
<dialog/>that should be present in the core block library.How?
Testing Instructions
Testing Instructions for Keyboard
The block toolbar controls should allow keyboard accessibility to open/close dialogs in the editor. And on the frontend the trigger is wrapped with a button element with proper arias to signal relationships. In both contexts there are keyboard handlers to handle escaping out of a dialog.
Screenshots or screencast
CleanShot.2025-09-11.at.13.24.28.mp4
CleanShot.2025-09-11.at.13.29.24.mp4