Replies: 6 comments 19 replies
-
|
Should the Header and Footer be sub-components? It's always felt a little odd to me that there's a prop to hide the header. This might enable a simplification (or even deprecation) of A dedicated footer component could allow for a more expressive design, so that we can do things like making the buttons fill the horizontal space on mobile: 
|
Beta Was this translation helpful? Give feedback.
-
|
I do agree with the sentiment. For the header and footer I think the main argument would be about ensuring consistency. IE all titles use the correct type styles, footers contain at most 2 actions, the mobile behavior in the screenshot above... those kinds of things. Also if we change details down the road (let's say we want to add a separator between content and footer) all modals receive the updates at once. |
Beta Was this translation helpful? Give feedback.
-
|
With a dedicated For example, if we want to always have the title and actions visible... 
<Dialog>
<Dialog.Header title="Alert!">
/* Any relevant controls... */
</Dialog.Header>
<p>Mauris blandit, arcu feugiat varius dapibus...</p>
<Dialog.Footer>
<Dialog.ActionButton
render={<Button variant="primary" />}
label="Oh no..."
onClick={ ... }
/>
</Dialog.Footer>
</Dialog>In general, I think I'd err on having additional sub-components if it makes consistency in the 90% of cases that are the same easier. The <Dialog.HeaderIconButton icon={ <IconClose /> } align="end" label="Close" onClick={ ... } />
// or
<Dialog.HeaderCloseButton />
// which internally maps to the same thing, and automatically closes the dialog.That doesn't stop consumers from doing this, if they still want to... <Dialog.HeaderCloseButton icon={ <IconBack /> } align="start" label="Back" />
<Dialog.HeaderIconButton icon={ <IconHelp /> } align="end" label="Help" onClick={ showHelp } />Equally, in the footer, we might provide an <Dialog.ActionButton render={<Button variant="secondary" />} label="Cancel" onClick={ ... } />
<Dialog.ActionButton render={<Button variant="primary" />} label="Confirm" onClick={ ... } />
// or
<Dialog.SecondaryActionButton label="Cancel" onClick={ ... } />
<Dialog.PrimaryActionButton label="Confirm" onClick={ ... } />
// or
<Dialog.ActionButton action={ Dialog.Actions.Cancel } />
<Dialog.ActionButton action={ Dialog.Actions.Confirm } />
// or
<Dialog.CancelActionButton />
<Dialog.ConfirmActionButton /> |
Beta Was this translation helpful? Give feedback.
-
|
@andrewhayward , In principle, I agree! However, from personal experience, this has not always been the case:
I remember seeing usages of Having said that, I think we could work towards a good compromise: having a set of header / footer sub-components that folks can decide to use in case they want to get maximum consistency. And in cases when the specific use-case requires a more custom layout, consumers can ditch those components and add their custom styles. |
Beta Was this translation helpful? Give feedback.
-
For this specific functionality, a Heading subcomponent (ie. title) should be enough. A header subcomponent could be mostly useful for spacing and layout (eg. action/dismiss buttons rendered in the header subcomponent are automatically rendered in the top right corner)
We would likely be able to implement this check strictly only if folks use the correct subcomponents (ie. Dialog.Footer and Dialog.FooterButton), but since we can't be guaranteed that folks won't use other custom components, we would probably at most be able to output a warning if we detect more than 2 Unless we want to have a super-strict footer element which doesn't allow custom
This is where the risk of this approach lies, IMO. In an ideal world, where all consumers of this component use it as it comes out-of-the-box, making such changes would be extremely easy and effective. But, out of experience, the reality in Gutenberg is that design needs change every week, and thus a component like this one would rapidly become unable to support all the different design specs. This means that folks would start applying style overrides, which ultimately make it really hard for us to make changes to the base component without breaking all those custom usecases. So, as also mentioned in my reply to Andrew above, a middle ground would be to offer a set of subcomponents that are not excessively opinionated, and that optional for folks to use. Not sure how everyone feels about it. |
Beta Was this translation helpful? Give feedback.
-
|
The compromise approach of useful-but-not-overly opinionated sub-components has definite merits. I agree with the value of easy ways to keep styling consistent. If we limit sub-component creation to just the things we expect most So it kind of feels like we want sub-components we think would get used a lot. But we want to avoid having them do too much, so they stay applicable to as many use-cases as possible. |
Beta Was this translation helpful? Give feedback.
-
|
I suppose this is tied to the conversation around separating header / footer into sub-components, but this seems like a good moment to discuss |
Beta Was this translation helpful? Give feedback.
-
|
If we were to create a new component,
I guess the main point of this conversation is to understand how much "design" we want to bake into |
Beta Was this translation helpful? Give feedback.
-
|
Yup it's a tricky one to balance. Consistency in design feels reasonably important to me, so I'd lean towards being more opinionated about the fundamental elements that 99% of modals use, IE sizing, header, footer, and general styling. I'm definitely open to being convinced otherwise though – not a strong feeling! |
Beta Was this translation helpful? Give feedback.
-
And looking at existing uses, I don't think it'll be missed. There may be third-party implementations that use it, but I don't actually see |
Beta Was this translation helpful? Give feedback.
-
Initial Sub-Component ProposalLooking at the above conversation, the key is going to be finding balance. Include simple sub-components that a high percentage of implementations will benefit from, but keep them simple so they're flexible and don't push consumers to start applying style overrides as they have to do with With that in mind, here's what I'm thinking, as a starting point. Feedback is very welcome, and we'll likely need to adjust and iterate on this list a bit as we go!
Notes:
|
Beta Was this translation helpful? Give feedback.
-
|
Some feedback:
|
Beta Was this translation helpful? Give feedback.
-
|
Would be nice if <Dialog title="Dialog Title" primaryActionLabel="Save" dismissActionLabel="Cancel">
<p>This is the content of my dialog.</p>
</Dialog>...would be functionally equivalent to... <Dialog.Root>
<Dialog.Heading>Dialog Title</Dialog.Heading>
<p>This is the content of my dialog.</p>
<Dialog.Buttons> {/* 🤷♂️ */}
<Dialog.DismissButton>Cancel</Dialog.DismissButton>
<Dialog.PrimaryButton>Save</Dialog.PrimaryButton>
</Dialog.Buttons>
</Dialog.Root>...or whatever the API ends up looking like. |
Beta Was this translation helpful? Give feedback.
-
That sounds great until someone needs to use Maybe a sweet spot could be to offer two versions of the component:
|
Beta Was this translation helpful? Give feedback.
-
I've actually been impressed by how well Ariakit has handled some of those out of the box. Andrew's example above would actually work well based on my initial experiments. The internal context does a good job of handling things like labelling and dismissal. Unless we encounter situations where some of these subcomponents need to render in a
The button aspect of this is tricky! When I think about it, it feels like the risk of making things overly-opinionated is especially high with this part of the component.
I think you're both on the same page here, unless I'm misreading, of having two versions: one monolithic and very simple to implement, the other composable and more flexible. I like that idea a lot, especially if we were to name them as Andrew described:
|
Beta Was this translation helpful? Give feedback.
-
Yes, I'm pretty sure we're saying the same thing differently. The namespaced sub-components are (mostly?) unstyled primitives, which can be used explicitly by developers as needed. We would essentially be exporting Ariakit's dialog implementation, with a nice wrapper around it for typical use-cases. |
Beta Was this translation helpful? Give feedback.
-
|
This is a good example of what I refer to when I say that our current |
Beta Was this translation helpful? Give feedback.
-
|
Chiming in here from #53852 (comment) I'd agree making the modal dialog easier to use and based on sub-components would be welcome. That said, the Besides cosntrained tabbing, making th erest of the content inert and non perceivable, closing on focus and the like, there is one feature that is fundamental for accessibility: the dialog needs to be labeled. We've had a few cases even in the editor where modal dialog were unlabelled. The new component must ensure the didalog is always labeled. Ideally, for better accessibility, the dialog should be labelled by the means of a visible H1 heading referenced with aria-labelledby on the element with The dialog ARIA pattern requires that:
For the sake of simplification, I'd propose that the dialog is always labelled with the H1 and Same applies to the default I think the most important thing to consider is that, while making the new component more flexible is reasonable, we also need to make sure WordPress provides accessible components that are not open to misues. Currently, some components are open to misues. For example, there have been several cases of unlabelled form controls and unlabelled modal dialogs. I'd really like all new components to avoid potential misues.
I'd agree but to some extent. The header / heading that provides the dialog labeling is arguably only 'content'. It is a required functionak part of the dialog that shouldn't be optional.
I'd argue that design often doesn't understand the modal dialog does have required features that can't be removed because, when removed, the accessibility level decreases. For example: styling the heading is perfectly legitimate. Visually hiding the heading is arguable and should be done only for very good reasons. Entirely removing the heading is a no-go. |
Beta Was this translation helpful? Give feedback.
-
|
I understand the point of view and agree with some of the principles, but I'm not sure if there's a good way of achieving this at the code authoring level without specific APIs for accessing the Accessibility Tree Object Model. For example, we could add a runtime check that either:
In case the check fails, we can output an error to the console. Although that would allow a consumer to still render the component. Also, we'd need to deal with a lot of edge cases, eg. what if the I'd be curious to know if and how |
Beta Was this translation helpful? Give feedback.
-
Ariakit has a
A If the |
Beta Was this translation helpful? Give feedback.
-
|
One option is to enforce the use of a |
Beta Was this translation helpful? Give feedback.
-
Thank you for sharing your thoughts. This has also been my stance around similarly related issues for the very same reasons, and I'm not sure exactly how a components library can successfully be "not open to misuse" as @afercia suggested above. In this specific case, an alternative would be, as Diego suggested, to have a required
|
Beta Was this translation helpful? Give feedback.
-
|
Couple of thoughts...
As an aside, these are valid points, but it's worth noting that this is intentionally not a
While I entirely understand the idea, from a technical perspective, this is not possible. There will always be ways of misusing components – we can require labels and descriptions, for example, and (though expensive) do runtime checks to assert constraints, but we can never validate that they are correct, relevant or useful. Of course, that is not to say that we shouldn't do our best to construct APIs that make correct usage easier, and minimise any potential issues. It might be reasonable to require interface Header {
title: string
// ...
};
interface Footer {
// ...
};
type Child = { id: string };
type Children = [...Child[], Footer | Child];
interface DialogBase {
// ...
}
interface DialogWithTitle extends DialogBase {
title: string,
children?: Children,
};
interface DialogWithHeader extends DialogBase {
title?: never,
children: [Header, ...Children],
}
type Dialog = DialogWithTitle | DialogWithHeader; |
Beta Was this translation helpful? Give feedback.
-
As also discussed in #65203, switching from conditionally rendering the component to having an |
Beta Was this translation helpful? Give feedback.
-
@WordPress/gutenberg-components is looking into what the next version of
Modalmight look like, and we're considering (not a final decision by any means) potentially creating new component rather than simply rewriting the existing one. There are two main benefits to a new component:Dialog)Tabswork, where we are introducing composable sub-components, and also allowing the component to be used in both controlled and uncontrolled modes.Our goal with this sub-component based approach is to create a component that is flexible rather than opinionated. It should semantically and accessibly implement the WAI-ARIA Dialog pattern. Or goal/recommendation is to create a
Dialogthat doesn't dictate what its contents should be, and then combine that with a set of relatively generic sub-components that can be used (or not used, as needed) by consumers.This approach would allow for more freedom and flexibility, so we'd like to gather feedback on what other improvements could be made over the current
Modalcomponent. What's missing fromModal? What's included inModalbut maybe doesn't work well? Or, are there any existing elements/behaviors ofModalthat we don't really need?Looking at some existing implementations of
Modal, in the editor, the component often gets a lot of style overrides applied to it. This indicates the default design isn't up to date and/or isn't flexible enough. What are the most important features to improve that experience?One area in particular that design input would be appreciated: When reviewing the current API surface of
Modalmany, if not most, props could be eliminated using sub-components as children, making the component simpler overall. If we think of a newDialogcomponent as a collection of composable and/or optional sub-components, what would the ideal structure look like? What sub-components would be most useful?One thing that is already on our radar is continuing to improve the preset sizing options in a way that gives consumers easy to use options, without becoming overly restrictive or opinionated. There have been a couple of different, but related conversations around this recently:
ModalHeight Controls #55062sizeprop with a fixed height #55130As work proceeds on this new component, we'll take those discussions into account and continue to iterate.
Changes the new component would likely need
There are a couple of bigger changes that we do think would be involved in a new component, based on initial analysis.
First,
Modalis rendered conditionally with the consumer managing the state that controls it. WithDialog, we'd use anopenprop instead, so the implementation would be a little different thanModalin that regard.Second,
Modalhas internal logic that's geared towards ensuring two non-nested modals cannot be opened at the same time. This isn't logic we think we'd like to carry into a new component, for a couple of reasons. First, rather than building a component that forces certain behaviors, we'd rather leave it flexible, and rely on consumers to render UIs properly and not trigger two modal dialogs at once. Also that's kind of tricky to do unless you really go out of your way to force it to happen.There will likely be other differences that surface as work continues, but before going further into the project, we wanted to get input from others on what this new component will need most.
cc @WordPress/gutenberg-design @richtabor @WordPress/gutenberg-components
(if you're reading this, please ping any other folks you think would have input to share!)
Beta Was this translation helpful? Give feedback.
All reactions