liferay
ClayModal: Deprecated the `useModal` hook
This issue is to raise ideas on how we can improve the Modal API and think about how we can replace the useModal hook or improve its API since some people don't like the way to create a Modal using the hook as a set or find it complex.
I've been thinking about this for some time, so some things we should take into account for an improvement of this API and start a deprecated phase if necessary.
One reason for the useModal hook:
- We need to control the animation when the visible state changes. We can do this inside the component without a
visibleAPI. - The components of
childrenneed to have access to theonClosemethod which is responsible for controlling the animation, settingvisiblemanually will interrupt the animation. We can allow this via rendering props.
<ClayModal>
{(onClose) => ...}
</ClayModal>
- You are not always setting the
visibleinside the component, so you need to set the visible outside the ClayModal. The case of render props does not cover this case. - We let the developer control the state of visible and conditional.
We have a few options that could kill the existence of the useModal hook but we wouldn’t be able to cover all the edge cases.
The idea of the observer is just to have an exclusive communication tunnel between the instances of hook and Modal, this allows you to have several through your application without any interfering with the other. Ideally, we should always have the only Modal (Although a mobile user interface is possible, natively, you can stack modal) but this does not seem to be a real case on the Portal 🤷♂️. If it were true we could have a Provider in the application that creates this communication tunnel and thus we could remove the use of the observer but we would still have the useModal hook.
We can remove the useModal hook by reconsidering a few things:
- We let the dev control the state of
visiblebut without having to add a conditional involvingClayModal. So we can remove the hook and internally handle the component with the animation of opening and closing the modal but the information ofvisiblewill not be true during the delay. - We can try to solve the animation problem on the CSS side to not need JS. @pat270 do you think that would be possible? Currently, we need to create a delay for Modal to appear on the page and then we add the
showclass. - Don't we use animation?
- Make
useModalauseStatewithout needing an additionaluseState.
// The visible here has a magic, it is not a `boolean` it would be
// the observer in the current case and it would also be null
// when the `visible` really was false.
const [visible, setVisible] = useModal();
return (
<>
{visible && <ClayModal visible={visible}></ClayModal>}
...
</>
);
Apparently we have several use cases just because of the delay in adding and removing the show class to create the animation. Using ClayModalProvider is a way we created to try not to use useModal or ClayModal directly, but only allows one Modal at a time. I'm just opening this speech to try to gather more different ideas and thoughts.
cc @markocikos @julien @wincent @jbalsas @kresimir-coko @bryceosterhaus I know that some of you got to use Modal and had some headaches, just checking if you want to leave your vision.
Personally I haven't had a ton of issues working with modal, although I also haven't used it a ton. I'm happy to switch up the API and try to make it more friendly. I would be curious where its been particularly painful for people.
Personally I haven't had a ton of issues working with modal, although I also haven't used it a ton. I'm happy to switch up the API and try to make it more friendly. I would be curious where its been particularly painful for people.
I also haven't used it (I've only had to touch it in code made by other people), but I have this picture of @julien talking about useModal at the office (back when we used to work in an office):
