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

add 'moveIn' and 'listNo' data attribute handler #90

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

Conversation

morganpizzini
Copy link

description here

@pjona
Copy link
Collaborator

pjona commented Oct 26, 2017

@morganpizzini Can you add some documentation about it in README.md?

@pjona pjona self-requested a review October 26, 2017 14:49
@RomanBurunkov
Copy link
Contributor

RomanBurunkov commented Oct 26, 2017

Consider updated README, I'm not quite sure that code:

            var itemMoveIn = this.dragEl.find(opt.itemNodeName).data('moveIn');
            var listNo = this.pointEl.closest('.dd-list').data('listNo');
            if (itemMoveIn !== listNo) {
                return;
            }

works properly with attributes: 'data-move-in' and 'data-list-no'.
It looks like attributes should be 'data-moveIn' and 'data-listNo'.

It's a bit confusing for me, but it works =)

@morganpizzini
Copy link
Author

ops yes, i think i press some strange shortcuts who delete "-" and uppercase the first char after 😄 sorry my bad, i'm going to fix it

@RomanBurunkov
Copy link
Contributor

@morganpizzini , that's, ok =)

But check my update to previous comment....it works =)))
Despite, fixing it also works and also more clear.

Copy link
Collaborator

@pjona pjona left a comment

Choose a reason for hiding this comment

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

@morganpizzini
Copy link
Author

Yes of course,
data-list-no inside "Item 6" must have 3 as index and not 2, same for data-move-in inside

https://jsfiddle.net/s5yt9mc4/7/

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

Successfully merging this pull request may close these issues.

3 participants