feat(Modal next): Introduce a next composable Modal

[tlabaj]

What: Closes #8076

A follow up issue will be needed once core issue #6091 is resolved. Until then the colors of the status icons in the header will be incorrect.

[patternfly-build]

Preview: https://patternfly-react-pr-9852.surge.sh

A11y report: https://patternfly-react-pr-9852-a11y.surge.sh

[thatblindgeye]

Several initial comments below. Let me know if anything would serve better as followups/if we mainly want to get the structure of the next implementation up.

[andrew-ronaldson]

Looks good to me. I filed a design-kit issue to add a custom header/footer variant to Figma for v5 and Penta.

[thatblindgeye]

Looks good! Some things that can still use updating, but we can open a followup issue to resolve these:

• We should think about automating the tabindex + role being applied on ModalBoxBody for when a scrollbar is rendered, similar to how Accordion is doing it
• Examples should be updated to remove the aria-describedby on the main Modal component unless there is a proper description passed into the ModalBoxHeader or there is another description element to point to. The Modal's aria-describedby prop should point to ModalBoxHeader's descriptorId prop (or ID of a similar description element) value instead (prop description should also be updated to omit mention of labelId).
• The ModalWithDescription example needs an aria-label passed into the ModalBoxBody to apply the region role
• Unresolved comment above regarding making the main Modal component functional instead of class based

[nicolethoen]

Couple quick questions -

• could we rename the ModalBoxBody and other sub components so they don't have the name Box? If they weren't exposed in the old implementation, i think it'd be clearer if they were just called ModalBody for example... I know a lot of the prop descriptions also reference the ModalBox, so if it's going to stick around, i think it needs an explanation somewhere in the composable documentation for what the ModalBox is and what it's used for.
• could we add the composable structure documentation like we do for the composable dual list selector documentation for clarity?

[tlabaj]

Couple quick questions -

• could we rename the ModalBoxBody and other sub components so they don't have the name Box? If they weren't exposed in the old implementation, i think it'd be clearer if they were just called ModalBody for example... I know a lot of the prop descriptions also reference the ModalBox, so if it's going to stick around, i think it needs an explanation somewhere in the composable documentation for what the ModalBox is and what it's used for.

@nicolethoen By "exposed" do you mean exported? Because they were exported in the old version

• could we add the composable structure documentation like we do for the composable dual list selector documentation for clarity?

Yes. that can possibly be a follow up if we want to get this in quicker.

[nicolethoen]

@nicolethoen By "exposed" do you mean exported? Because they were exported in the old version

Ah - they were not referenced in the old documentation as far as I could tell.
And they still are only referenced in prop descriptions in the new implementation, so i don't think there is any explanation of their purpose anywhere.

I think that any references to 'box' need more context if they need to stay there.

[tlabaj]

@nicolethoen By "exposed" do you mean exported? Because they were exported in the old version

Ah - they were not referenced in the old documentation as far as I could tell. And they still are only referenced in prop descriptions in the new implementation, so i don't think there is any explanation of their purpose anywhere.

I think that any references to 'box' need more context if they need to stay there.

@nicolethoen We purposely did not expose docs for the other exported components because the modal was not designed to be composable. I am not opposed to removing "box". I thought about it. I ended up just leaving them as is since most they were already being exported. I believe I originally named them with box for the old Modal to match core componet names.

[mcoker]

LGTM! Only thing we might update is the double close button on the modal w/ wizard example. Also left a couple of comments about alternatives to the utility classes in case you'd prefer remove those from the examples.

And just to confirm, the alert/status variants are waiting on this core issue, then we'll update the examples so the status icon is assigned the correct color? patternfly/patternfly#6091

^ nevermind, just saw your note in the PR description. I put up a PR for the core change patternfly/patternfly#6180 - here's a link to a new example with the status modifier on the title https://patternfly-pr-6180.surge.sh/components/modal#danger-alert-with-modifier-on-modal-title

[mcoker]

This is causing a layout issue with the modal's close button and the wizard. Doesn't look like you need this, the wizard's close button closes the modal.

[mcoker]

An aside, but looks like this PR is adding the "beta" label to the existing modal - both in the vertical nav and the example page title/header area on https://patternfly-react-pr-9852.surge.sh/components/modal

Not sure if that's on purpose?

[tlabaj]

An aside, but looks like this PR is adding the "beta" label to the existing modal - both in the vertical nav and the example page title/header area on https://patternfly-react-pr-9852.surge.sh/components/modal

Not sure if that's on purpose?

Yes, I noticed that to. I opened this issue in org

[nicolethoen]

I won't hold this up only because of the question about using 'box', since we can iterate on this in the future.
I still think that without any explanation about the purpose or definition of the 'box' related components, that any off handed references to them in the docs don't seem to make much sense.
Maybe it's worth a follow up issue?

[mcoker]

🤖👍