react-hooks-use-modal icon indicating copy to clipboard operation
react-hooks-use-modal copied to clipboard

use of dialog tag for modal structure ?

Open Nayte91 opened this issue 3 years ago • 12 comments

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 avatar Aug 30 '22 17:08 Nayte91

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 avatar Aug 30 '22 17:08 Nayte91

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 avatar Aug 30 '22 17:08 Nayte91

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 avatar Sep 28 '22 12:09 Nayte91

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 :

  • useModal() // useNonModal() ?
  • useModal() // useDialog() ?
  • useModalDialog() // useNonModalDialog() ?
  • or having a parameter as an object with position for non modal dialog; if missing, then it's a modal (centered on screen)

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 avatar Dec 08 '22 11:12 Nayte91

About the usability of tag on browsers

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

  • On Safari under 15.4, it doesn't work well. Safari is now 15.6, so I confirm your ~1% global
  • On Firefox, works very well since at least v100.0, and the backdrop works well, I don't know why this table says it's not, maybe a bug? I opened a ticket but I closed it since, because they corrected their table about ::backdrop, it works very well so it was a mistake on their side!
  • On Chrome & Edge, everything is fine since at least v100.0, and when I try to tab in a modal, the focus stays trapped, I switch between my only 3 buttons and my address bar,
  • On Opera, everything is fine since at least v85.0,

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.

Nayte91 avatar Dec 08 '22 11:12 Nayte91

@Nayte91 Hi!

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

  • Demo https://codesandbox.io/s/long-resonance-bks9lf?file=/src/App.tsx

  • Source https://github.com/dc7290/react-hooks-use-modal/blob/feature/dialog-element/src/future/index.tsx

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

  • Have to provide a focus-trap.  - Bugs in re-rendering due to it
  • Decrease in the number of ModalWrapper dependencies returned by useModal
  • z-index considerations

dc7290 avatar Dec 10 '22 16:12 dc7290

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!

Nayte91 avatar Jan 09 '23 16:01 Nayte91

@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 avatar Jan 18 '23 08:01 dc7290

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

dc7290 avatar Jan 25 '23 03:01 dc7290

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!

Nayte91 avatar May 02 '23 19:05 Nayte91

@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.

dc7290 avatar May 10 '23 07:05 dc7290

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.

Nayte91 avatar May 12 '23 10:05 Nayte91