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

feat: let DragDropContextProvider reuse a DragDropManager #1362

Merged

Conversation

gustavohenke
Copy link
Contributor

@gustavohenke gustavohenke commented May 30, 2019

tl;dr: this PR gives DragDropContextProvider an extra prop, manager, which will be reused instead of creating a new DragDropManager.


Full explanation:
I'm trying to load react-dnd conditionally in our app.
For mobile devices, for example, it doesn't make sense to have the same dnd interactions, so the chunk with this code doesn't need to be loaded at all.

I could almost do this... however I hit a wall: if react-dnd finishes loading a few seconds after the whole app is ready, the user will notice React remounting the whole app, because I had to reparent it with the DragDropContext.

To avoid this, each component with dnd interactions will be wrapped in a DragDropContextProvider, which unfortunately creates a new DragDropManager -- not ideal.

This PR then lets this component receive a manager prop, which will be reused instead of creating a new DragDropManager when rendered.
I can do this once in my app by calling createDragDropManager at the app's bootstrap level, and then passing it around with my own React context 🙂


Unfortunately I'm not sure how to go about docs here.
dnd-core is undocumented, so I'm guessing any attempts of documenting the change to react-dnd would require documenting details about dnd-core (I'm happy to do it with some ideas).

@ishwortimilsina
Copy link

ishwortimilsina commented May 31, 2019

I have been scratching my head over (what seems like) a similar problem for a few days now.

My issue :
I want to pass DragDrop context (along with the AppContext) to components rendered outside of the tree using ReactDOM.render (not ReactDOM.createPortal).

  • I cannot combine the dragDropManager from the original DragDropContextProvider with the AppContext because the dnd "Consumer" won't recognize it.
  • I cannot use another DragDropContextProvider to wrap the "child" component again because it will then introduce multiple HTML5backends which is not allowed.
    AppContext is the primary context which I can easily consume in any part of my application.

Do you think your change will help me address this issue? It looks like it will.

@gustavohenke
Copy link
Contributor Author

gustavohenke commented May 31, 2019

Hi @ishwortimilsina, so my implementation will be something along these lines:

export const MyDropTarget = () => (
  <MyContext.Provider>
  {myContext => {
    <DragDropContextProvider manager={myContext.manager}>
      <MyInternalDropTarget/>
    </DragDropContextProvider>
  })
  </MyContext.Provider>
);

Since I want to achieve asynchronous loading of react-dnd, it will be way more complex than that, but you get the picture. So yeah, maybe it will help you!

@darthtrevino darthtrevino merged commit 884c6ff into react-dnd:master Jun 4, 2019
@gustavohenke gustavohenke deleted the drag-drop-manager-reuse branch June 4, 2019 08:17
@ahackettms
Copy link

@gustavohenke have you solved the issue of loading of react-dnd asynchronously? Does this pull request help achieving that? Thanks

@gustavohenke
Copy link
Contributor Author

Hi @ahackettms, I haven't continued working on this yet.
However, yes, this would be the missing bit for our implementation -- my comment above (#1362 (comment)) should outline what it should look like

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

Successfully merging this pull request may close these issues.

4 participants