Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with nested NiceModal Provider #100

Open
quangphuchuynh95 opened this issue Jan 19, 2023 · 6 comments
Open

Problem with nested NiceModal Provider #100

quangphuchuynh95 opened this issue Jan 19, 2023 · 6 comments

Comments

@quangphuchuynh95
Copy link

Steps to reproduce

  1. Wrap the whole app with a NiceModal.Provider
  2. Then use react-router or simple state to create 2 page
  3. On page 1 use NiceModal.Provider to wrap all children
  4. Add a button and simple Modal for page 1
  5. On page 2 don't wrap it with a NiceModal.Provider
  6. Add a button and simple Modal for page 2
  7. Add buttons to switch between pages
  8. Start the app

Case 1:

  • Go to page 1 directly then click the button to open the dialog -> it works
  • Go to page 2 by clicking the button we add in step 7 above then click the button to open the dialog -> it doesn't work

Case 2:

  • Go to page 2 directly then click the button to open the dialog -> it works
  • Go to page 1 by clicking the button we add in step 7 then click the button to open the dialog -> it works
  • Go to page 2 by clicking the button we add in step 7 then click the button to open the dialog -> it doesn't work

Github repository

https://github.com/quangphuchuynh95/test-nice-modal

Codesandbox

https://codesandbox.io/s/wizardly-goldberg-rp3vtz?file=/src/index.js

@quangphuchuynh95 quangphuchuynh95 changed the title Problem with nested NiceModalProvider Problem with nested NiceModal Provider Jan 19, 2023
@alexandredev3
Copy link
Contributor

Why are you wrapping PageAContainer with NiceModal.Provider again? Didn't you already wrap your entire app with NiceModal.Provider in index.js?
Just remove NiceModal.Provider from PageAContainer, and all cases should work.

@quangphuchuynh95
Copy link
Author

quangphuchuynh95 commented Feb 10, 2023

@alexandredev3 That's a good question.
Let me explain a bit deeper in my case. I have a code example here.

My app structure:

<ThemeProvider value={globalTheme}>
  <NiceModal.Provider>
    // some other levels of children
      <ThemeProvider value={localTheme}>
        <MyComponent />
      </ThemeProvider>
    // ...
  </NiceMpdal.Provider>
</ThemeProvider>

MyComponent:

// ...
NiceModal.show(ModalComponent)

return ...

ModalComponent:

// ...
const theme = useTheme() 
// ...

Here it will return globalTheme instead of localTheme, even though <MyComponent /> is mounted inside the <ThemeProvider value={localTheme}>
I understand because ModalComponent is mounted at a <PlaceHolder> at the same level of <NiceModal.Provider>
So I need to add another <NiceModal.Provider> inside <ThemeProvider value={localTheme}>, to make the localTheme accessible from <ModalComponent />

@alexandredev3
Copy link
Contributor

alexandredev3 commented Feb 11, 2023

Well, if I'm not wrong, you can't have a NiceModal.Provider and wrap its children with another NiceModal.Provider. Probably because the API gets lost between the NiceModal providers, and there's no way to say if the show method that you're calling is from the first Provider or the second one. To fix that, you'd have to work around this by isolating the first NiceModal.Provider and all its children and doing the same with the second NiceModal.Provider and all its children.

Something like this:

<ThemeProvider value={globalTheme}>
  // First NiceModal Provider
  <NiceModal.Provider>
    <ComponentChildren />
  </NiceModal.Provider>

  // Second NiceModal Provider
  <NiceModal.Provider>
    <ThemeProvider value={localTheme}>
      <MyComponent />
    </ThemeProvider>
  </NiceModal.Provider>
</ThemeProvider>

@supnate supnate closed this as completed Oct 2, 2023
@supnate supnate reopened this Oct 2, 2023
@kainstar
Copy link

kainstar commented Feb 6, 2024

I have a suggestion about how to support nested NiceModal Provider:

Give NiceModal.Provider a optional user defined id prop, and user can declare which provider the show method should render modal to, something like this:

<ThemeProvider value={globalTheme}>
  // First NiceModal Provider, id is "default", it will use globalTheme
  <NiceModal.Provider>
    <ComponentChildren />
    // Second NiceModal Provider, id is "local", it will use localTheme
    <ThemeProvider value={localTheme}>
      <NiceModal.Provider id="local">
        <MyComponent />
      </NiceModal.Provider>
    </ThemeProvider>
  </NiceModal.Provider>
</ThemeProvider>

// static API to show
NiceModal.show(CustomModal, {
  provider: 'local', // if not set, it will use "default"
  // other options
})

If want automatic use nearest provider, use a new hooks to wrap it

const provider = NiceModal.useProvider()

// static API
NiceModal.show(CustomModal, {
  provider: provider.id,
  // other options
})

// hooks API
provider.show(CustomModal)

@supnate
Copy link
Collaborator

supnate commented Feb 6, 2024

Thanks @kainstar ! I was thinking about a similar solution (not tried yet), use some concept like mountNode or ModalPlaceHolder where modal is rendered.

Something like,

const mountNode = useRef()
NiceModal.show(modal, props, mountNode.current)
...

The benefit is no need for nested wrapper provider. The disadvantage is the holder can only be defined where modal is used. It's very like using declarative way.

So I've not decided it.

@kainstar
Copy link

kainstar commented Feb 6, 2024

mountNode sounds like specifying the rendering position of the modal in the actual DOM tree. In my opinion, we write nested ModalProvider is because we want it to use different context value, such as theme configuration. The rendering position of the modal component in the DOM tree is always implemented by UI libraries.

The ModalPlaceholder sounds similar to antd's useModal, which specifies where the modal component is rendered in React's virtual DOM tree and allows it to access all context information at that location. However, when writing big application, it's boring to write template code to obtain and render placeholders for each component that would renders modals. Provide a solution to use placeholder in parent component is more efficient (like antd's useApp).

Overall, I believe ModalPlaceholder might be a better option, and it would be even better if official support nested providers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants