LPD-50827 New Variant of Clay Modal

[BarbaraCabrera]

Motivation

Due to this story we are contributing a new Variant of Clay Modal to be used in the CMS and with Marketplace in DXP.

Changes

To do so, I have added the “clean” attribute that hides the title and changes the styles to not show the header and keep the close button in place. Also it adds a "sr-only" to the title.

Once we have this, to obtain the modal that we want to use in both cases, we would only have to place the image, the title and the desired text in the body manually.

[pat270]

I couldn't find an example of a modal with an image in the Figma document. The way it looks is similar to Cards. We can add a status type called asset at https://github.com/liferay/clay/blob/master/packages/clay-modal/src/types.ts#L6 instead of clean here. It will add the class modal-asset to modal-dialog.

We're using asset because the card with image is named card-type-asset in Clay CSS. We can scope all the styles to .modal-asset.

[BarbaraCabrera]

Looks good, I think it's a good idea to do it from the status. Thanks!

[pat270]

We should scope all styles in .modal-asset.

.modal-asset {
  .modal-header {
    height: auto;
    ...

    .close {
       position: absolute;
       ...
    }
  }

  .modal-body {
    ...
  }
}

[pat270]

This demo doesn't make sense. There should be an image in the header?

[BarbaraCabrera]

Our proposal is to put it in the body. Are you proposing to put the image in the header for some specific reason that we don't know?

[marcoscv-work]

It's probably better in the body, as it seems like adding it to the header adds it to the <h1>, but we don't want to add the semantic information of the image to the title

[pat270]

I see, putting everything in body is fine too. You will run into an issue where there will be no close button if you don't include an empty <ClayModal.Header><ClayModal.Header/>. We would need to implement a close button on ClayModal.Body.

[marcoscv-work]

That's why the idea was to make the .modal-title sr-only and then float the close button. The conceptualization exercise was done here: https://codepen.io/marcoscv/pen/azbwEeV
But guys, you know best how to adapt these exercises to a real-world clay implementation!

[pat270]

@marcoscv-work I think it's better to have an empty <ClayModal.Header><ClayModal.Header/> and don't output h1 if empty. We can absolute position the close button.

[marcoscv-work]

Hey @pat270, There is no particular reason why we want to keep the header but one of the reasons why our modal is accessible is because when the focus goes to the modal, the screen reader can read the h1 as a label of the modal itself, it's something we need to keep even if the title is not visible. This way, we still have a semantic heading when we have open modals:

If we take that into account, and provide the h1 in another way and correctly related to the modal by aria-labelledby, everything should be OK

[matuzalemsteles]

I don't like this approach because you can achieve this with the current component using composition. You can get something close to that.

You can achieve the same thing with this example but with a bit of CSS, maybe what we can add here is to allow the floating close button.

const {observer, onOpenChange, open} = useModal();

if (!open) {
    return null;
}

return (
  <Modal observer={observer}>
    <Button
      aria-label="close"
      className="close new-class-to-make-floating"
      displayType="unstyled"
      onClick={() => onOpenChange(false)}
    >
      <Icon symbol="times" />
    </Button>
    <Modal.Body>
      Content
    </Modal.Body>
    <Modal.Footer
      last={
        <ClayButton.Group spaced>
          <Button
            displayType="secondary"
            onClick={() => onOpenChange(false)}
          >
            Cancel
          </Button>
          <Button onClick={() => onOpenChange(false)}>
            Save changes
          </Button>
        </Button.Group>
      }
    />
  </Modal>
);

[marcoscv-work]

I don't like this approach because you can achieve this with the current component using composition. You can get something close to that.

You can achieve the same thing with this example but with a bit of CSS, maybe what we can add here is to allow the floating close button.

const {observer, onOpenChange, open} = useModal();

if (!open) {
    return null;
}

return (
  <Modal observer={observer}>
    <Button
      aria-label="close"
      className="close new-class-to-make-floating"
      displayType="unstyled"
      onClick={() => onOpenChange(false)}
    >
      <Icon symbol="times" />
    </Button>
    <Modal.Body>
      Content
    </Modal.Body>
    <Modal.Footer
      last={
        <ClayButton.Group spaced>
          <Button
            displayType="secondary"
            onClick={() => onOpenChange(false)}
          >
            Cancel
          </Button>
          <Button onClick={() => onOpenChange(false)}>
            Save changes
          </Button>
        </Button.Group>
      }
    />
  </Modal>
);

We need the header title <h1> always present to be read as main semantic information, the modal title then needs to be visually hidden, like an sr-only.
This is important to understand you are opening a modal with a specific purpose, and the <h1> is read as soon as the modal is opened and the focus is moved to the modal.
If this is possible to do without customizations, that would be perfect, but we need a way to add the sr-only class to the <h1>/ header.

[BarbaraCabrera]

I don't like this approach because you can achieve this with the current component using composition. You can get something close to that.

You can achieve the same thing with this example but with a bit of CSS, maybe what we can add here is to allow the floating close button.

const {observer, onOpenChange, open} = useModal();

if (!open) {
    return null;
}

return (
  <Modal observer={observer}>
    <Button
      aria-label="close"
      className="close new-class-to-make-floating"
      displayType="unstyled"
      onClick={() => onOpenChange(false)}
    >
      <Icon symbol="times" />
    </Button>
    <Modal.Body>
      Content
    </Modal.Body>
    <Modal.Footer
      last={
        <ClayButton.Group spaced>
          <Button
            displayType="secondary"
            onClick={() => onOpenChange(false)}
          >
            Cancel
          </Button>
          <Button onClick={() => onOpenChange(false)}>
            Save changes
          </Button>
        </Button.Group>
      }
    />
  </Modal>
);

The reason to contribute it is not to use custom CSS and that it will be used both in the CMS and DXP in the marketplace integration with the page-management components to replace the existing one (modules/apps/layout/layout-js-components-web/src/main/resources/META-INF/resources/js/components/modals/MarketplacePresentationModal.tsx).

Here is an example of use in the CMS
And here appears in the figma of the story

[matuzalemsteles]

@marcoscv-work @pat270 @BarbaraCabrera I see, we can still do it with composition without needing to render the Moda.Header additionally because we won't need it. So we can still use the Modal.Title which will still render the h1 along with the id for the dialog so we can add the sr-only to it and render the button later. I think we can create a new class for this button case.

<Modal observer={observer}>
    <Modal.Title className="sr-only">Modal Title</Moda.Title>
    <Modal.Body>
        <Button
            aria-label="close"
            className="close new-class-to-make-floating"
            displayType="unstyled"
            onClick={() => onOpenChange(false)}
        >
            <Icon symbol="times" />
        </Button>
      Content
    </Modal.Body>
</Modal>

What do you think?

[pat270]

Wouldn't the modal title already be in the body? In this image:

the modal title would be "Marketplace is now in Page Builder".

[marcoscv-work]

@pat270 It's not necessarily the title of the modal, but it could be. If so, it would have to be related via aria-labelledby.
We are evaluating other uses like the example in the codepen.

If you guys think the current component can handle those compositions, that would be perfect!. If you could provide detailed examples, that would also be great, and a good example of the component flexibility!

[matuzalemsteles]

I think the way we can go about this is to replicate these use cases in the storybook with the examples I showed and I also think we can add some CSS to make the close button floating in this case.

@pat270 @marcoscv-work what do you think?

[marcoscv-work]

Sounds perfect! thanks @matuzalemsteles @pat270
cc @BarbaraCabrera

[BarbaraCabrera]

Then I guess I can close this task and you guys can take care of it from now.
If so, thanks a lot guys! 😄

[pat270]

@matuzalemsteles @marcoscv-work sounds good