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

Update to Node 10 + dependencies #221

Merged
merged 1 commit into from
Jun 9, 2018
Merged

Update to Node 10 + dependencies #221

merged 1 commit into from
Jun 9, 2018

Conversation

beefchimi
Copy link
Contributor

@beefchimi beefchimi commented May 23, 2018

@beefchimi beefchimi added examples specific to the examples directory greenkeeper automated dependency management labels May 23, 2018
@beefchimi beefchimi self-assigned this May 23, 2018
@beefchimi beefchimi requested a review from tsov May 23, 2018 01:44
@@ -61,9 +61,6 @@ export default function DragEvents() {
});

draggable.on('drag:move', (evt) => {
// Required to help restrict the draggable element to the container
evt.cancel();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tsov I don't know how this was working for so long... but drag:move is not cancellable... thing is, at one point this worked and it prevented the "Pill Switch" from being pulled outside of the "Track".

Is there a similar trick I can use here?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I will check this branch out later and see what's up, drag:move should be a cancelable event https://github.com/Shopify/draggable/blob/master/src/Draggable/Draggable.js#L492

height: rows(6);
// suddenly breaking in Chrome...
// subtrack a single pixel to fix the issue
height: rows(6) - 0.1rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Layout broke... I don't know why. This was the fix.

Copy link
Member

@tsov tsov left a comment

Choose a reason for hiding this comment

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

Let's resolve all issues first, then we can merge this 👍 I wonder, should we be merging this into the beta branch?

@@ -61,9 +61,6 @@ export default function DragEvents() {
});

draggable.on('drag:move', (evt) => {
// Required to help restrict the draggable element to the container
evt.cancel();
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I will check this branch out later and see what's up, drag:move should be a cancelable event https://github.com/Shopify/draggable/blob/master/src/Draggable/Draggable.js#L492

@beefchimi
Copy link
Contributor Author

@tsov

drag:move should be a cancelable event

Yea I thought so as well, as clearly at one point it was, but when it stopped working in this latest update, I went and looked at the README and it says "cancelable: false"

https://github.com/Shopify/draggable/tree/master/src/Draggable/DragEvent#dragmoveevent

That is really the only issue I'd want you to look into. The rest I can take care of, and if not, I'll reduce the scope of this PR. Might not get around to it until after vacation.

@beefchimi beefchimi changed the title Update to Node 10.1.0 + dependencies Update to Node 10 + dependencies Jun 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples specific to the examples directory greenkeeper automated dependency management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants