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

Task Hierarchy: drag-and-drop task from one part of the hierarchy to another #37

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

4xdk
Copy link
Collaborator

@4xdk 4xdk commented Feb 24, 2019

#2

Using the PR submitted at https://github.com/PursuanceProject/pursuance/pull/195/files as base.

Added the debounce (from lodash which btw. has a lot of nice functions I think would be nice to use all around) to limit canDrop calls as per: https://github.com/PursuanceProject/pursuance/pull/195/files#r197622886

And added the caching of parsed creation date as per: https://github.com/PursuanceProject/pursuance/pull/195/files#r197625840

Copy link
Member

@elimisteve elimisteve left a comment

Choose a reason for hiding this comment

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

Please make these tweaks @4xdk and then I think we're good 👍 . Let's both look at how often these functions are run, but I think the lodash.debounce fixes the original issue from before. Thanks!

);
const newSubtaskGids = [...newParentTask.subtask_gids, taskGid];
const newSubtasks = newSubtaskGids.filter(
(gid, idx) => newSubtaskGids.indexOf(gid) === idx
Copy link
Member

Choose a reason for hiding this comment

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

Every subtask gid has the same index as itself... or am I reading this wrong?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reading well ... I'll remove this bit.

@@ -1,6 +1,8 @@
import React, { Component } from 'react';
import { connect } from 'react-redux';
import { BrowserRouter as Router, Route, Switch } from 'react-router-dom';
import { DragDropContext } from 'react-dnd';
import HTML5Backend from 'react-dnd-html5-backend';
Copy link
Member

Choose a reason for hiding this comment

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

Let's make sure there's no negative privacy repercussions here. Is HTML5 the only back end option that exists for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, the only other one is the TouchBackend which doesn't seem to work using the mouse (one could say, obviously).

The thing is HTML5 backend doesn't work on touch devices (again, obviously).

I've checked the TouchBackend on iPhone6 (browserstack) and drag and drop seems to work ... sometimes. I guess it's and IOS issue: https://github.com/yahoo/react-dnd-touch-backend/issues/78. Although Android (Pixel 3) seemed to work even worse (I managed to delete some tasks trying to drag and drop 👎)

Anyway, if we're still up for it we could use https://louisbrunner.github.io/dnd-multi-backend/packages/react-dnd-multi-backend/
Quote: "HTML5toTouch starts by using the React DnD HTML5 Backend, but switches to the React DnD Touch Backend if a touch event is triggered. You application can smoothly use the nice HTML5 compatible backend and fallback on the Touch one on mobile devices!"

But as far as your original question goes re. privacy repercussions. No, I don't think we have any other backend option with react-dnd for desktop. Is HTML5 a privacy problem?

@@ -24,6 +24,13 @@ import {
deleteMembershipReq,
} from '../api/memberships';

export const moveTask = (oldParentGid, newParentGid, taskGid) => ({
type: 'MOVE_TASK',
Copy link
Member

Choose a reason for hiding this comment

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

How about MOVE_TASK_IN_HIERARCHY? Soon we may be reordering task lists in My Tasks

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.

None yet

2 participants