microcmsio / react-hooks-use-modal

This is a customizable modal with react-portal.
https://microcmsio.github.io/react-hooks-use-modal/
171 stars 21 forks source link

use of dialog tag for modal structure ? #49

Open Nayte91 opened 2 years ago

Nayte91 commented 2 years ago

Hello,

To begin with, thank you very much for your work on this hook. I'm going mad with react and the absurd difficulty of doing a simple HTML modal, I tried to do my component, and spent a lot of time on tutorials and repositories; For now, I may say that yours is by far the most satisfying out there.. Or at least the one my own style is the most aligned with :)

As my journey begun with the new

tag - its new behavior - I was wondering if you already considered your Modal component around this tag ?

If not, let me give you some materials:

  • The dialog element comes with an interesting behavior, as it's hidden by default,
  • You can "open" it by calling 2 dedicated js functions, show() for a modalless behavior (you can click out, etc), or showmodal() for a strict user catch. I will continue around the second.
  • When one of those is used, an "open" attribute appears in the dialog element, and some properties change like making it visible, centered, on top of the Z pile. Also it's easy to style with like #modal-structure[open] { display: grid }
  • When showModal() is used, a new backdrop pseudo-element appears and makes it easy to style the background,

On a structure like yours, the dialog tag use can saves you both <div style={wrapperStyle}> and <div style={overlayStyle} />, quite interesting.

Nayte91 commented 2 years ago

The tricky part with React is that is not a thing to let a DOM node (a component) to live here, being hidden. But we need to fire the JS's showModal() function to enjoy the properties.

I found a little trick (I don't know how many rules it violates but..) to allow to display the modal with something like a boolean useState variable (isOpen) and fire the showModal() function: with 2 async functions and an awaited operation into.


const ModalStructure = forwardRef((props, ref) => {
    const [isOpen, setOpen] = useState(false)
    const modalRef = useRef()

    useImperativeHandle(ref, () => ({
        showModal: async () => {
            await setOpen(true)
            modalRef.current.showModal()
        },
        closeModal: async () => {
            await modalRef.current.close()
            setOpen(false)
        },
    }))

    return isOpen ? (
        <dialog
            id="modal"
            ref={modalRef}
            onClose={() => ref.current.closeModal()}
        >
            <header className="modal__header">
                <h1>Hello</h1>
                <button
                    className="close-btn"
                    type="button"
                    onClick={() => ref.current.closeModal()}
                />
            </header>
            <main>{props.children}</main>
        </dialog>
    ) : null
})```
Nayte91 commented 2 years ago

I think you have here all the valuable parts of my researchs, as the others (custom hook, portal, ...) are way better on your side.

If you want to give it a try, I'ld be happy!

Nayte91 commented 2 years ago

Hello @dc7290! I hope you are fine,

Following our discussion around my POC PR, I was concerned about your question: "For now, I have a feeling that the inconvenience of applying styles when using the dialog element may be a disadvantage.",

An elegant answer can be found in the :modal CSS pseudo class that makes styling convenient from outside of the component.

It actually has some support, that you can take in consideration.

Have a nice day,

Nayte91 commented 1 year ago

I put here the discussion and ideas instead of in a pull request:

Hello @dc7290 ,

Wonderfull! I will be the first user and fan of this new version!

  1. Unfortunately you are way stronger than me on React, so I can't really help technically,
  2. I think my PR with Proof of Concept has the core idea in it, especially those lines:
    await setOpen(true);
    document.getElementById('dialog-poc').showModal();

    Beyond that, I can only give advices/wishes about the global API, what can be enjoyable to use.

Few ideas

Hook's API

As there is non-modal dialog elements and modal ones, I think we need to provide 2 custom hooks, where names must be choose :

I feel like this kind of pattern is a bit annoying and can be optimized:

const Test = () => {
    const {Dialog, openDialog} = useDialog()

    return (
        <>
            <div>
                <h1>My actual component</h1>
                <p>Lorem ipsum</p>
                <button onClick={() => openDialog()}>Click for modal</button>
            </div>

            <Dialog>
                <MyModalContent />
            </Dialog>
        </>
    )
}

I wanted to do such a thing:

const Test = () => {
    const {openDialog} = useDialog(<MyModalContent />)

    return (
        <div>
            <h1>My actual component</h1>
            <p>Lorem ipsum</p>
            <button onClick={() => openDialog()}>Click for modal</button>
        </div>
    )
}

But it seems that the 'MyModalContent' must output somewhere it can be rendered, and I can't in a hook. Maybe with a global DialogContext?

Styling the modal

For the style, I think the easiest way to apply is:

  1. to allow user to inject style in hook,
  2. document clearly what can be styled and how,

example of style injection:

import styles from './test.module.css'
import Modalstyles from './modal.module.css'

const Test = () => {
    const {openDialog} = useDialog(<MyModalContent />, Modalstyles)

    return <h1 className={styles.title}>Hello</h1>
}

But it works because I use CSS modules, with nextJS. Maybe this something to try?

Nayte91 commented 1 year ago

About the usability of tag on browsers

I did some tests with my browserstack (stayed on Laptop MacOS):

As you said the main concern is the cutting-edge Safari version for users.

As the focus trap, isolation, centering, keyboard are managed, I feel like <dialog> is a far better starting point than <div>. Even if it needs some further improvements and brings few limitations (i.e. default backdrop zone can't be clicked to close a modal), build over <dialog> seems to simplify a lot the hook.

Here's a good article about how to build around dialog.

dc7290 commented 1 year ago

@Nayte91 Hi!

At least I made something that works, so I'm sharing!

I'm able to solve a lot of problems I was having using the DIALOG element.

Nayte91 commented 1 year ago

Hello @dc7290,

It works well! I like how it seems simple to use. Also the style's injection seems sweet.

I wonder if bodyscrolllock still relevant, as if you choose between ModalDialog (JS showModal()) or NonModalDialog (JS show()) it will define this behavior?

Anyway It's a great draft, and I'm impressed how fast you made it!

dc7290 commented 1 year ago

@Nayte91

Thanks for looking!

I wonder if bodyscrolllock still relevant, as if you choose between ModalDialog (JS showModal()) or NonModalDialog (JS show()) it will define this behavior?

Indeed!!! This is not necessary lol.

dc7290 commented 1 year ago

@Nayte91 It has been available since v3.3.0! https://github.com/microcmsio/react-hooks-use-modal/releases/tag/v3.3.0

Nayte91 commented 1 year ago

Hello! I hope you are fine. And of course, many thanks for the feature. I'm using it on my NextJS project. Do you use it?

Just wanted to share with you this video from a famous frontend Youtuber who speak about

. I'm happy that we are some months before him :)

Fortunately there is some nice tricks, especially about how to add a parameter to close the modal dialog when clicking outside. Can be a neat implementation!

dc7290 commented 1 year ago

@Nayte91 It has been a long time! How are you?

I see! Nayte91 had the foresight to see this:)

I am curious about its implementation. I am waiting for Nayte91's contribution lol.

Nayte91 commented 1 year ago

Hello! I'm really fine, I hope you are too!

I actively use the lib and the new modal on my NextJS project :) Unfortunately I'm more a backend dev (PHP/Symfony) and I'm struggling with JS & React; I can dev a bit with, but contributing is harder.

Noneless I will try this one in june ;) I have the target code, I may succeed in it. I'll keep you in touch ! Waiting for Street Fighter 6 release, launching an app for this occasion, then I will have some time to dive into this.