Clay: ClayModal, how to close it properly

[julien]

I've got a question regarding the new @clayui/modal component.

I've set up an example in codesandbox here and I'd like to know what I doing wrong.

If you look at the example and click on the "Close Me" button, you'll notice that a warning is displayed in the console.

Note that, this warning only happens once, even if you have multiple modal components in your "app".

This wasn't happening before 3.0.0.

I also commented this line (in my local node_modules directory) and the warning disapears.

Any help is appreciated, thanks.

[diegonvs]

Yes, this is weird. Needs more investigation but We found a temporary walkaround. If you move useModal from the Modal component to your App component this error doesn't appear more. https://www.diffchecker.com/3AXpWjOT

[bryceosterhaus]

First off, thanks for the codesandbox, always a huge help.

Hmm I think it might have to do with the useModal hook being in a component that is unmounted at the same time as the modal itself. I refactored it slightly here so that useModal exists outside of that mounted component, and it doesn't seem to error.

However, this does seem like a bug. I would have thought that line wouldn't even run since it should be cleared when the hook is unmounted here at this line. https://github.com/liferay/clay/blob/d0a2e3659a8b0f56f02bc3a2ca9fc2c35ccfe6ad/packages/clay-modal/src/useModal.tsx#L64

I'll label this as an issue and we can look into it.

[bryceosterhaus]

looks like me and @diegonvs were both looking at the same time lol

[matuzalemsteles]

hey @bryceosterhaus, @diegonvs yeah I think we have to also analyze that even correcting this, if this case would be the most recommending to close the modal. The hook may be lost every time the component is unmounted at the same time as Modal, due to the use of the above ternary in the example. For me it makes more sense for the hook to be out of ternary since it is Modal's controller, it needs to be listening to Modal's changes since it is observer.

[julien]

Thanks to all for the quick reply.

@bryceosterhaus based on your updated example moved the useModal hook into the parent component (the one that renders the modal component) and it did indeed remove the warning so thanks for the fix.

[julien]

Hi again,

I'm able to fix most of our current use cases by moving the useModal hook inside the modal's parent component, but what I haven't been able to do is use this in tests - We're currently using react-testing-library and I can't see how I could "mock" this inside my tests. Just wanted you to be aware of this because the modal component is widely used in various applications.

[julien]

Just a(nother) quick update, I finally managed to mock the observer in the tests with this code

const observer = {
  dispatch: (type) => {},
  mutation: [1, 1]
};

It's a big ugly but it works ;)

[matuzalemsteles]

@julien Thanks for bringing this up, can you give us some context in the tests?

Just a(nother) quick update, I finally managed to mock the observer in the tests with this code

haha please keep this at seven keys just for you 😂. This is an API that I don't want people to know how it works. 😅

[julien]

@matuzalemsteles sure.
I'll try to explain this simply: by moving the useModal hook into the modal's parent component, the modal component now requires an observer prop so that it can pass it to it's <ClayModal> child.

e.g.

function MyModal({
  observer,
  {...otherProps}
})  {
   return (
    <ClayModal observer={observer} size="md">
    { /* Children */ } 
   </ClayModal>
   );
}

But there were also tests using react-testing-library that were rendering the component like this

import {render} from '@testing-library/react';

function _renderMyModal(props) {
  return render(
    <MyModal {...props} />,
    {
      baseElement: document.body
     }
  );
}

So in this case, I can't use the useModal hook otherwise, React would tell me (which seems logical)

    Invariant Violation: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
    1. You might have mismatching versions of React and the renderer (such as React DOM)
    2. You might be breaking the Rules of Hooks
    3. You might have more than one copy of React in the same app
    See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.

The only way around this that I could think of was to "mock" the observer.

This is an API that I don't want people to know how it works.

I understand, but the fact that developers have to use the useModal hook each time they use the modal component, makes them want to use it; at least that's what I've observed so far.

In some cases, and just so you know, I've seen things like:

const visible = observer.mutation;

Because in some cases, you need to know when the modal is "visible" in order to focus one of it's elements, etc...

I hope that helps. ;)

[matuzalemsteles]

Thanks a lot again @julien! I talked a little with Julien in Slack, so these are some of the thoughts I had quickly so we could encourage some practices and try to remove some that were already created.

• For testing, we can encourage the practice of creating a wrapper (<ModalWithState />) on top of useModal and Modal, as we do in our tests. We might want to document this on clayui.com
• For the correct use of useModal, we can better document this on clayui.com and address the use cases.
• We know when Modal is really visible, we can create an API to not encourage direct access to observer, maybe dispatch a warning when someone tries to access it directly.

[victorg1991]

hey @matuzalemsteles I had to use the observer directly because I needed to focus one element when the modal is visible. It seems that the modal is rendered twice before it is first shown (probably due to the React portal stuff) and the only reliable way of knowing when the modal is visible is tracking the observer mutation property

Is there another way to handle this case?

[matuzalemsteles]

hey @victorg1991 no problem about that for now, you could take some approaches:

• Create the expert Modal without the useModal hook and without the conditional rule attached to the component and add auseEffect with dependency [] so that it runs only on that component mount, with focus logic.

const ModalWithFirstFocus = ({children, ...otherProps}) => {
    useEffect(() => {
        // Add focus logic
    }, []);

    return (
        <ClayModal {...otherProps}>
            {children}
        </ClayModal>
    );
};

Let's add useModal props so you can know when the Modal content really became visible. Both renderings happen due to two changes, the initial render that will cause the animation and when the animation finishes another render that is to show the content. This was necessary due to some issues reported here #2342.

[bryceosterhaus]

Going to close. If you still need questions answered I can re-open.