Implement Drag-n'-Drop support #19
Conversation
|
Did unfortunately not work for me on linux (tested i3 and openbox). Maybe you could test this again on you system. The linked issue mentions opening new lumper instanced for multiple files. |
Works fine for me on Windows-- I've yet to try on my Linux install, but I'll get around to it after work today.
I can look into doing this. |
Liar, liar, pants on fire!
AvaloniaUI/Avalonia#6085 Appears to be a known issue. That's fine for now.
It works now. Just a little slow (perhaps the best option will be to delay open until after the new window is open so the user thinks something's happened) |
Riven-Spell
force-pushed
the
feat/drag-n-drop
branch
3 times, most recently
from
ebf1a4c
to
486cf53
Compare
There was a problem hiding this comment.
Implementation looks okay, works for me on Windows. We definitely need to show some indicator when stuff in loading but that's a separate thing and requires async refactor that BIRD is writing up an issue on now.
Then a few general housekeeping issues:
- Your editor seems to have changed a bunch of line ending or something, meaning loads of lines of changes that shouldn't be there.
- Your branch includes merge commits, which we don't allow. This is our fault really, we don't have good guidelines for this repo. I've been drafting some general contribution guidelines this afternoon that'll be on the organisation README in the future. In the meantime, here's a preview:
- Keep your branch up-to-date by rebasing, not using merge commits. We consider merge commits to be a major detriment to the quality of a Git repo, and will always ask you to remove any merge commits in code review, even if this means force-pushing.
- Keep your commits clean and simple, splitting all changes into separate commits.
- Don’t push commits specifically for addressing issues in earlier commits, including those brought up in code review. Instead, make those commits then fixup them into the original commit using an interactive rebase. Again, we have no problem with force-pushes; so long as you make sensible local backups, we feel that rewriting history of non-master branches is a necessary evil to avoid merge commits. For a general guide on this workflow, see here.
- Name your commits using conventional commits (no Gitmojis!). If in doubt, just check the rest of the history of the repo and copy that.
It's probably easiest to address these changes, then just reset --mixed and recommit everything.
Riven-Spell
force-pushed
the
feat/drag-n-drop
branch
4 times, most recently
from
4d2bad1
to
1832b25
Compare
Riven-Spell
requested review from
tsa96 and
BIRD311
and removed request for
tsa96 and
BIRD311
|
Looks good to me! @BIRD311 did you bring up the issue about the file location or whatever you were talking about yesterday? |
Riven-Spell
force-pushed
the
feat/drag-n-drop
branch
from
1832b25
to
80ea252
Compare
Riven-Spell
force-pushed
the
feat/drag-n-drop
branch
from
80ea252
to
6bc8b50
Compare
Riven-Spell
force-pushed
the
feat/drag-n-drop
branch
from
6bc8b50
to
5c9e67c
Compare
| @@ -1,5 +1,6 @@ | |||
| using Avalonia; | |||
| using Avalonia.Controls.ApplicationLifetimes; | |||
| using Avalonia.Input; | |||
| @@ -17,10 +18,12 @@ public override void OnFrameworkInitializationCompleted() | |||
| { | |||
| if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime | |||
| desktop) | |||
| { | |||
There was a problem hiding this comment.
just revert this file so the drag and drop commit only touches file it has to
| @@ -163,7 +163,7 @@ private async Task LoadBsp(IStorageFile file) | |||
| if(!file.TryGetUri(out var path)) | |||
| { | |||
| throw new Exception("Failed to get file path"); | |||
|
|
|||
There was a problem hiding this comment.
same here, revert or move formatting stuff to a separate commit
|
This could be picked up again once my PR is in, either by the original author if they're still around, or someone else. But I'd suggest just rewriting / copying stuff over rather than rebasing. The DataContext shadowing stuff I don't understand, just use ReactiveWindow (might be doing this alrwsy on my PR, don't remember) and WhenActivated. And setting up the event handlers should be done in the view. |
Resolves #18
This is probably on the hacky/bad side, due to my general lack of familiarity with Avalonia-- But, I'm happy to hear any and all feedback, even if it's "Start from scratch and do it this way". I'm here to learn and contribute.