Skip to content
This repository has been archived by the owner on Jun 6, 2020. It is now read-only.

Separate drop events #69

Merged
merged 3 commits into from
May 7, 2020
Merged

Separate drop events #69

merged 3 commits into from
May 7, 2020

Conversation

zbaylin
Copy link
Member

@zbaylin zbaylin commented May 7, 2020

As discussed in revery-ui/revery#833, it makes sense to differentiate between events that carry a file/text/path and those that don't.

This splits dropEvent into dropEvent and dropNotificationEvent. dropNotificationEvent is for when SDL is simply notifying that a drop is about to/has taken place through either DROPBEGIN or DROPCOMPLETE. dropEvent is for when there is information about the actual items being dropped.

This is a very simple change, but I definitely agree with both @glennsl and @bryphe that it improves clarity for these events!

Copy link
Member

@glennsl glennsl left a comment

Choose a reason for hiding this comment

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

Looks great! Seems to even make the implementation here simpler!

I think you might have gone a bit too hard on the lockfiles though.

src/sdl2.re Outdated Show resolved Hide resolved
Co-authored-by: Glenn Slotte <glenn@slotte.net>
@zbaylin
Copy link
Member Author

zbaylin commented May 7, 2020

Looks great! Seems to even make the implementation here simpler!

I think you might have gone a bit too hard on the lockfiles though.

Yeah not sure what happened there. That was just the result of me running esy install 🤔. Is there anything you think I should do?

Copy link
Member

@bryphe bryphe left a comment

Choose a reason for hiding this comment

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

Looks great, Zach! Thanks for going the extra mile and updating this API. Sorry I missed this reviewing it the first time around!

@bryphe
Copy link
Member

bryphe commented May 7, 2020

Will publish this once it is in 👍

@zbaylin
Copy link
Member Author

zbaylin commented May 7, 2020

Sounds good! I'll wait for this to pass CI and then I'll merge it in!

@glennsl
Copy link
Member

glennsl commented May 7, 2020

Yeah not sure what happened there. That was just the result of me running esy install thinking. Is there anything you think I should do?

./update-lockfiles.sh usually fixes these kinds of issues

@zbaylin
Copy link
Member Author

zbaylin commented May 7, 2020

Yeah not sure what happened there. That was just the result of me running esy install thinking. Is there anything you think I should do?

./update-lockfiles.sh usually fixes these kinds of issues

Does that exist here? Using find doesn't turn up anything.

@glennsl
Copy link
Member

glennsl commented May 7, 2020

Hmm, I guess it doesn't. There's only one other sandbox here though, so just running esy @js install should do it then. Hopefully.

@zbaylin
Copy link
Member Author

zbaylin commented May 7, 2020

Hmm, I guess it doesn't. There's only one other sandbox here though, so just running esy @js install should do it then. Hopefully.

No go 😕. The git status after running that is empty 🤔.

@zbaylin
Copy link
Member Author

zbaylin commented May 7, 2020

Deleting the lock directories and running esy i again also produces the same lockfiles, so I think it's okay..? Not sure what else I would do.

@glennsl
Copy link
Member

glennsl commented May 7, 2020

I guess so. Weird. If any dependency had been updated to drop a bunch of dependencies I'd expect esy.lock/index.json to have changed as well.

@zbaylin zbaylin merged commit 0432c9e into master May 7, 2020
@zbaylin zbaylin deleted the events/fix-drag branch May 7, 2020 21:53
@bryphe
Copy link
Member

bryphe commented May 7, 2020

Published 2.10.3028 👍

@zbaylin zbaylin changed the title Separate drag events Separate drop events May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants