New Modal API Changes

[thejackshelton]

What is it?

This PR proposes a couple of developer experience improvements to the modal.

In a previous release, the following components have been deprecated:

• ModalHeader
• ModalContent
• ModalFooter

These components were native header, div, and footer elements and did nothing special under the hood. We are deprecating them in order to simplify the API and make it more consistent with the rest of the components.

The new components are:

<Modal.Root>

This is the main container of the modal, and now holds the major props and configuration. Examples include:

• 'bind:show'?: Signal;
• closeOnBackdropClick?: boolean;
• alert?: boolean;
• onShow$?: QRL<() => void>;
• onClose$?: QRL<() => void>;

<Modal.Panel>

Previously <Modal /> the modal panel is the dialog element that is rendered on top of the page. Its props have since been moved to the <Modal.Root /> component, please refer to the docs for more information.

<Modal.Trigger>

The modal now comes with a default trigger, which will open the modal when clicked.

<Modal.Title>

This computes the accessible name from the string children of the modal.

<Modal.Description>

This computes the accessible description from the string children of the modal.

• Feature / enhancement
• Bug
• Docs / tests
• Other

Why is it needed?

Checklist:

• My code follows the developer guidelines of this project
• I have performed a self-review of my own code
• I have ran pnpm change and documented my changes
• I have add necessary docs (if needed)
• Added new tests to cover the fix / functionality

[changeset-bot]

🦋 Changeset detected

Latest commit: 130c5f4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@qwik-ui/headless	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[maiieul]

Shouldn't the ModalTrigger button also have some aria attributes on it?

Like in Radix:

const DialogTrigger = React.forwardRef<DialogTriggerElement, DialogTriggerProps>(
  (props: ScopedProps<DialogTriggerProps>, forwardedRef) => {
    const { __scopeDialog, ...triggerProps } = props;
    const context = useDialogContext(TRIGGER_NAME, __scopeDialog);
    const composedTriggerRef = useComposedRefs(forwardedRef, context.triggerRef);
    return (
      <Primitive.button
        type="button"
        aria-haspopup="dialog"
        aria-expanded={context.open}
        aria-controls={context.contentId}
        data-state={getState(context.open)}
        {...triggerProps}
        ref={composedTriggerRef}
        onClick={composeEventHandlers(props.onClick, context.onOpenToggle)}
      />
    );
  }
);

Also I feel like if we add a ModalTrigger we'll need to add an asChild prop to it. This way users can directly put their <Button> component in the ModalTrigger.

Edit: implementing the asChild pattern doesn't seem to be very easy. Here's a good article on how to do it in React/Svelte/Vue/Solid. There must be a way to do it in Qwik as well. Probably the Vue way is the closest.

[thejackshelton]

Yes, the following aria attributes should be added to the modal trigger, thanks for pointing that out.

As for the asChild pattern, I don't see the large benefit. The component needs to know its direct child, which goes against the philosophy of both slots and Qwik's out of order rendering.

To be consistent, then half the library would need an asChild prop, which lengthens the markup. I'm not sure having an inline component everywhere would be the best idea either.

I don't see the article you've linked. In either case, a trigger is more the most part a button. There are two reasons for the switch:

1. A better developer experience and a11y as a default.
2. Invokers. https://open-ui.org/components/invokers.explainer/

[maiieul]

Lol sorry, forgot to link it: https://medium.com/@bryanmylee/aschild-in-react-svelte-vue-and-solid-for-render-delegation-645c73650ced

[maiieul]

WDYT about having a Modal.Close? Could be convenient in order to not have to do a signal = false manually.

[thejackshelton]

WDYT about having a Modal.Close? Could be convenient in order to not have to do a signal = false manually.

I saw it in Radix, but didn't want to add it in until I saw a particular need for it. After looking through the examples, I'll go ahead and add this to the PR, as I think it would improve the developer experience.

[maiieul]

Hmmm I guess in any ase if we need so aria attributes on the Modal.Trigger then we need this component.

Also having a Modal.Rootmeans we can simply do the bind:show={isOpen} on it and no need to play with the signals in the other components on a general basis.

As for the asChild prop, if it means we must create an inline component for it, then perhaps it's best to address that in a future PR.

So I guess LGTM 👍