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

Switch from react-simple-dnd to react-dnd. #705

Merged
merged 1 commit into from
Oct 23, 2017
Merged

Switch from react-simple-dnd to react-dnd. #705

merged 1 commit into from
Oct 23, 2017

Conversation

tech4him1
Copy link
Contributor

- Summary

react-simple-dnd is just a wrapper for react-dnd, and it doesn't support React 16. This PR changes to using the underlying library directly.

- Test plan

Manually tested unpublished entries drag-and-drop cards.

- Description for the changelog

Switch from react-simple-dnd to react-dnd.

- A picture of a cute animal (not mandatory but encouraged)

@tech4him1 tech4him1 changed the title WIP: Switch from react-simple-dnd to react-dnd. Switch from react-simple-dnd to react-dnd. Oct 16, 2017
@tech4him1 tech4him1 closed this Oct 17, 2017
@tech4him1 tech4him1 reopened this Oct 18, 2017
@decaporg decaporg deleted a comment from codecov-io Oct 18, 2017
@tech4him1 tech4him1 requested a review from erquhart October 18, 2017 03:19
Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tech4him1 just to keep this parseable, we should at least split out the internals so that the implementation in UnpublishedListing is about the same. We could keep the drag and drop primitives in a directory under src/components/UI.

Thoughts?

@tech4him1 tech4him1 force-pushed the react-dnd branch 7 times, most recently from e5d156d to a859c40 Compare October 22, 2017 22:27
`react-simple-dnd` is just a wrapper for `react-dnd`, and it doesn't
support React 16. This PR changes to using the underlying library
directly.
Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, looks good!

@@ -11,6 +11,9 @@ import Button from 'react-toolbox/lib/button';
import UnpublishedListingCardMeta from './UnpublishedListingCardMeta.js';
import { status, statusDescriptions } from '../../constants/publishModes';

// This is a namespace so that we can only drop these elements on a DropTarget with the same
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@erquhart
Copy link
Contributor

Going to go ahead and merge since this is mostly a port and fairly straightforward (and blocking the React 16 update).

@erquhart erquhart merged commit cd72e1d into master Oct 23, 2017
@erquhart erquhart deleted the react-dnd branch October 23, 2017 15:59
@tech4him1 tech4him1 mentioned this pull request Oct 23, 2017
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.

2 participants