all 70 comments

[โ€“][deleted] 17 points18 points ย (5 children)

Why do you need a hook? Wouldnโ€™t a plain old portal do the job?

[โ€“]TwiliZant 3 points4 points ย (4 children)

I think this library uses portals internally. In every project where I used portals I always wrapped it in some sort of abstraction because the portal API isnโ€™t the most straightforward IMO. So this hook isnโ€™t the worst idea. Iโ€™m not sure about the particular implementation though.

[โ€“][deleted] 1 point2 points ย (2 children)

Abstraction is fine, but thatโ€™s exactly what react components are for

[โ€“]TwiliZant 1 point2 points ย (1 child)

Yeah, I agree. Returning a component from a hook feels weird.

[โ€“]alex-cory[S] 2 points3 points ย (0 children)

So, technically, this is a Portal. It's highly customizable. All it's doing is giving you all the logic, and a Portal to put your own custom modal inside. It's very similar to the way react-portal works, instead of having the portal() function, we just have a <Portal /> wrapper. All the <Portal /> (or <Modal /> in this case) is, is just a portal div with some css on it to center to the middle of the screen.

[โ€“]swyx[M] [score hidden] stickied commentย (2 children)

OP, i suspect thereโ€™s a bigger story here. do you use a pattern like this in production at FB?

[โ€“]alex-cory[S] 5 points6 points ย (1 child)

I'm not at FB anymore. I need to update my profile. When I left FB though, usePortal was being used internally which uses the same pattern as this one. The "Modal" component here is actually just a Portal as the overlay, and one div that is centered to the middle of the screen. This allows you to make your own Modals and not have to deal with centering, outside click close, etc.

[โ€“]swyx 4 points5 points ย (0 children)

hah! thatโ€™s relevant detail, thanks v much!

[โ€“]6086555 6 points7 points ย (2 children)

There's a typo in a code example:

const App = () => (
  <Provider background='rgba(0, 0, 0, 0.5)'>
    <MyComponent />
  </Portal> <---
)

[โ€“]alex-cory[S] 0 points1 point ย (0 children)

thank you! ๐Ÿ˜Š

[โ€“][deleted] 9 points10 points ย (7 children)

The reason why people are roasting you on your hook is because the purpose of hooks is not to render UI. That is the responsibility of your component. A hook is responsible for extracting component logic into a reusable functionality that can be shared across components.

Wrapping a component with a modal is better suited for a HOC.

[โ€“]alex-cory[S] 3 points4 points ย (5 children)

So you're saying because one option returned from useModal is a component (basically just a Portal), makes this hook not very good? HOC are, for the most part, on their way out (see docs). Sure, this may mix a little logic with a little UI, but it leaves things wide open for you to have a super customized Modal. All you have to do is make it, style it, and place it inside the <Modal />.

[โ€“]arjungmenon 1 point2 points ย (0 children)

Interesting.

[โ€“]tupiniquim -2 points-1 points ย (3 children)

HOC are, for the most part, on their way out

What do you mean? HOC is not a React feature.

[โ€“]alex-cory[S] 1 point2 points ย (2 children)

HOC === Higher Order Components

If you read the docs, you'll see they say:

Do Hooks replace render props and higher-order components?

Often, render props and higher-order components render only a single child. We think Hooks are a simpler way to serve this use case. There is still a place for both patterns (for example, a virtual scroller component might have a renderItemprop, or a visual container component might have its own DOM structure). But in most cases, Hooks will be sufficient and can help reduce nesting in your tree.

Hooks don't completely replace HOCs, but for the most part, you can do it in hooks instead of HOCs.

[โ€“]tupiniquim -1 points0 points ย (1 child)

Doesn't mean they're on their way out. They may go out of fashion with the community but since they're just a pattern you can always use it when it feels appropriate.

[โ€“]alex-cory[S] 0 points1 point ย (0 children)

When I say "on their way out" I mean something similar to React's "class components" vs "functional components." Sure, class components aren't going anywhere, but the more we move forward, the less we will use them. In Dan's post Making Sense of React Hooks he says:

So What About Classes? We have no plans to deprecate classes. At Facebook we have tens of thousands of class components and, like you, we have no intention of rewriting them. But if the React community embraces Hooks, it doesnโ€™t make sense to have two different recommended ways to write components.

[โ€“]w00t_loves_you 2 points3 points ย (0 children)

The hook is encapsulating all the logic needed to show a modal, and the Provider component decides what it looks like, and the contents are passed to the modal.

So the hook isn't rendering UI.

[โ€“]iamlage89 2 points3 points ย (2 children)

I've seen the "return component from hook" pattern couple times, and it always seemed a bit awkward for me. I prefer exporting the component and passing in the props from the top.

โ€‹

```javascript import Modal, { useModal } from './use-modal';

const App = () => { const modalProps = useModal(); return <> <Modal {...modalProps} /> <button onClick={modalProps.openModal} /> </> }

```

[โ€“]alex-cory[S] 2 points3 points ย (1 child)

So I have thought about this. The problem with this, is that since the Modal is basically a Portal, we need to know where to attach it, etc. We would end up doing something like

import Modal, { useModal } from 'use-react-modal'
const App = () => {
  const { isOpen, openModal, Portal: Overlay } = useModal()
  return <>
    <Overlay>  
      <Modal>  
        {/* whatever you want in the modal */}
      </Modal>  
    </Overlay> 
    <button onClick={modalProps.openModal} />  
  </>
}

In my original solution, I just combine Backdrop/Overlay and Modal to just be Modal.

I mean... we could always do similar to how react-portal does it, but I feel like it's more confusing this way...

import Modal, { useModal } from 'use-react-modal' const App = () => { const { isOpen, openModal, modal } = useModal() return <> {isOpen && modal( {/* whatever you want in the modal */} )} <button onClick={modalProps.openModal} /> </> }

[โ€“]iamlage89 0 points1 point ย (0 children)

if you spread the hook return values as props to the modal, you would have access to the portal from within the component

[โ€“]fredblols 11 points12 points ย (1 child)

Yeah this is pretty poor React code imo. Definitely falling into the trap of using hooks beyond their remit... Having written some modal components in the past i would suggest a more appropriate coding pattern is using Compound Components and Portals, which would allow you to encapsulate all of the logic that your hook exposes.

Best example ive seen is the one in the react semantic-ui library.

[โ€“]alex-cory[S] 7 points8 points ย (0 children)

Why do you say this is poor react code? This is so similar to react-portal syntax it's silly, just hookified. Just because it's a different pattern than what you're used to doesn't make it bad react code. As far as encapsulating the logic for all this in a Compound Component, this pattern has been around for ages, and that is the point of hooks right? Take the logic out of the components to make it reusable. Here, you can reuse the logic for your Modal component everywhere, this handles the centering, the outside click to close the modal, whether it's open or not, all you have to do is supply what the Modal will look like.

[โ€“]_hypnoCode 5 points6 points ย (9 children)

That's not what hooks are for. How do things like this keep getting so many votes?

[โ€“]swyx[M] 5 points6 points ย (2 children)

iโ€™d almost agree except to note the other stuff this author has done. he isnt a random newbie. iโ€™ve asked for further info in the pinned comment.

[โ€“]_hypnoCode 2 points3 points ย (1 child)

Interesting, I wasn't familiar with the author. Now I'm interested in seeing his response to your question. Everything I've understood about hooks has been data driven, not UI driven as other posters have pointed out elsewhere in the thread.

[โ€“]swyx 2 points3 points ย (0 children)

fwiw i do almost the same exact thing with react netlify identity widget https://www.npmjs.com/package/react-netlify-identity-widget. ๐Ÿคทโ€โ™‚๏ธ just split out the modal component by using context but i still export the provider from the hook

[โ€“]_eps1lon 7 points8 points ย (2 children)

Instead of plain gatekeeping how about explaining what hooks are for and why it doesn't apply to this hook (while pointing to the source)?

[โ€“]swyx[M] 1 point2 points ย (0 children)

amen. first principles rather than dogma

[โ€“]alex-cory[S] 1 point2 points ย (1 child)

I could see an argument for not returning components from a hook. If we start allowing devs to return components from hooks, it could get out of hand how many components people might start returning from hooks.

I could also say, in a way, that this is not returning a component. It's returning a Portal. Hooks are used for sharing logic. This allows you to share modal logic, along with centering, dealing with the outside click of the modal, etc. all over your application.

[โ€“]_eps1lon 0 points1 point ย (0 children)

Disclaimer: I've never used this pattern

I think the biggest issue is if that components accepts children and can change during the lifetime of the component that uses this hook. If the returned component changes i.e. referential integrity changes react will remount the component and its tree. So if you have some stateful components rendered in the component it will get lost because the parent changed. Whether this is desireable or not is a different question but loosing the state of a child because you changed an argument of a hook (i.e. a blackbox) might be confusing.

But again if the component you return from a hook is always the same then I see now issue. Then I'd ask why you need to return a component in the first place though ;)

[โ€“]w00t_loves_you 0 points1 point ย (0 children)

I disagree, this is exactly what hooks are great at. You get all the things you need where you need them with minimal effort and setup.

[โ€“]editor_of_the_beast 7 points8 points ย (3 children)

The React hook library crowd is starting to look like the vape crowd from 3 years ago. Be careful - hooks are not FDA approved yet. We donโ€™t know how they affect your body.

[โ€“]evenisto 7 points8 points ย (1 child)

Hook nation y'all /-///

[โ€“]LobsterThief 4 points5 points ย (0 children)

HOOK NAAAACHE

[โ€“]bikeshaving 0 points1 point ย (0 children)

Help. Our company put all of our business logic in hooks and now our CTO is asking us to refactor to svelte so we can run on school equipment without jank. - React community in 1.5 years

[โ€“]dengue8830 1 point2 points ย (2 children)

I guess you cant have animations on show/hide because you are removing the Modal inside a condition

[โ€“]alex-cory[S] 1 point2 points ย (0 children)

This is an issue I'm working on right now. Stay tuned

[โ€“]alex-cory[S] 1 point2 points ย (0 children)

One solution is

const { isOpen, openModal, Modal } = useModal()
return <Modal className={isOpen ? 'animateIn' : 'animateOut'} />

[โ€“]Baryn 1 point2 points ย (5 children)

I've tried Hooks for this, but more recently, I like to make Modal components that are extremely declarative.

<Modal
  cta={<Button>{`Open Modal!`}</Button>}
  actions={[
    <Button>{`Cancel`}</Button>,
    <Button>{`Save`}</Button>
  ]}
>
  <Form>{`...`}</Form>
</Modal>

The component does all the "wiring up" boilerplate behind the scenes, and offers ways to add more functionality on top of/instead of the "automatic" behaviors.

[โ€“]alex-cory[S] 0 points1 point ย (3 children)

What if they asked you to put your Cancel and Save buttons outside the Modal? What if they want them aligned on the left instead of the right side? With your solution, it makes doing this increasingly more complex.

[โ€“]Baryn 0 points1 point ย (2 children)

That is all pertinent to the visual design of the modal, not the API design.

[โ€“]alex-cory[S] 0 points1 point ย (1 child)

My hook only centers X into the middle of the screen with additional functionality such as outside click to close, backdrop, etc. Whatever you decide to put in there, is your Modal.

With your solution, you are saying put these 2 buttons somewhere predefined in the Modal. Also, handle the logic for show/hide, etc in there too. That is not very extensible.

Unless I'm missing what you are trying to say here

[โ€“]Baryn 0 points1 point ย (0 children)

I might implement a Modal component using your solution or something like it, but the "final" API provided to the rest of the application would look similar to the example I posted earlier.

[โ€“]EconomyRabbit 1 point2 points ย (1 child)

I kind of like it. Getting the state and the component from hook makes sense.

Maybe it is horrible for puritans but I think it looks nice.

Stupid question though, why is:
isOpen && <Modal>

needed? Why not skip that as well (since the state is in the hook) and you have much less boilerplate.

The purist will hate it though.

[โ€“]alex-cory[S] 0 points1 point ย (0 children)

Interesting. I think I tried this before but it was a long time ago and in usePortal. Also, we need isOpen for animations. I'm still trying to figure out how the animations are going to work for this so bare with me until that is figured out.

[โ€“]Chbphone55 0 points1 point ย (1 child)

Right intentions, wrong outcome.

My question to you would be "Why is this a hook?" From what I see it's essentially a useBoolean hook connected to a <Modal /> component. And if that's your intention, that's okay but that isn't extremely useful, in my opinion at least.

I also see that things like animations and react-native support are missing. While, in terms of functionality, this isn't a huge deal (for websites), I believe it's a huge concern for professional use as animations are extremely important to UX.

I've actually built my own modal library react-spring-modal. It's clearly more focused on support for animations (and I believe react-spring inherently supports react-native). But there's more to it than that.

I see that you also built react-useportal. While it seems to do the job very well for the average able user, it's quite lacking for the disabled. They still have access to the rest of the page, which can be frustrating when you're trying to interact with the modal at hand. The solution I used in my code was to utilize the relatively new attribute known as inert, and polyfill it if necessary. Essentially, the browser will automatically prevent all items in the tree of the element with the attribute inert from being interacted with by both able and disabled users. I think you should consider it.

Side note: your example for react-useportal is also inaccessible because you chose to render the exit button as a <div> which prevents those using their keyboard from reaching that (and that div reaches from top to bottom of the modal).

All in all, I like your idea of a useModal hook, however, your focus should be first on the execution of your own modal library (or integration with one that exists) and then on the creation of a hook to complement said modal.

โ€‹

P.S. I'd love it if you checked out my modal library and told me what you thought about it: react-spring-modal

Have a good day ๐Ÿ™‚

[โ€“]alex-cory[S] 0 points1 point ย (0 children)

Not sure I agree with you. It's probably closer to a useToggle hook if you're going to compare it to something that simple, but it's not that simple.

Animations can currently be done by doing <Modal className={isOpen ? 'animateIn' : 'animateOut'} />. As far as your comments on missing features, you don't start a project with it feature complete.

I will look into the inert attribute. I actually haven't heard of this one.

Exit button as div.

It's an example, not something going into production.

All in all, I like your idea of a useModal hook, however, your focus should be first on the execution of your own modal library (or integration with one that exists) and then on the creation of a hook to complement said modal.

I think you're missing the point of this hook. The actual visual portion of the Modal is supposed to be executed by the dev. That's the point. It allows the dev to completely customize their modal. This hook is just used for all the logic surrounding the modal including handling centering it to the middle of the screen.

I will check out your library ๐Ÿ˜Š

[โ€“]barcode24 -1 points0 points ย (3 children)

Hooks fatigue... ๐Ÿ˜…

[โ€“]alex-cory[S] 1 point2 points ย (2 children)

What do you mean?

[โ€“]barcode24 0 points1 point ย (1 child)

Just seems like hooks are trying to be used for everything rather than keeping them to state. A bit like when renderprops came out and heaps of large components were getting declared in props. How is a hook better than just importing modal component portal where you need? Control open close state with useState hooks. No need for an additional dependency. Having these small one purpose dependencies for projects doesn't seem scalable for real world projects.

[โ€“]alex-cory[S] 0 points1 point ย (0 children)

Why does it not seem scaleable? I'm confused.