-
Notifications
You must be signed in to change notification settings - Fork 66
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
React to drag and drop #602
React to drag and drop #602
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #602 +/- ##
==========================================
- Coverage 76.83% 76.83% -0.01%
==========================================
Files 106 106
Lines 10049 10022 -27
Branches 1658 1651 -7
==========================================
- Hits 7721 7700 -21
+ Misses 1796 1793 -3
+ Partials 532 529 -3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a host is offline, draging a save file for upload will instead prompt the user to download the zip file. (tested on firefox)
The idea I had was that it would show you the drop targets when you drag a file anywhere over the Web UI so that you could see this table accepted it without having to know in advance that you had to drag it over the table. Basically the logic for determining if a file was dragged would be something like this function useIsDraggingFile() {
const [isDraggingFile, setIsDraggingFile] = useState<boolean>(false);
const elements = useRef(new Set());
const enter = (e: DragEvent) => {
elements.current.add(e.target);
if (elements.current.size === 1 && e.dataTransfer?.types.includes("Files")) {
setIsDraggingFile(true);
}
}
const leave = (e: DragEvent) => {
elements.current.delete(e.target);
if (elements.current.size === 0) {
setIsDraggingFile(false);
}
}
const end = (_e: DragEvent) => {
elements.current.clear();
setIsDraggingFile(false);
}
const over = (e: DragEvent) => {
// Needed to make drop event to work
e.preventDefault();
}
const drop = (e: DragEvent) => {
// Prevent opening link or downloading the dragged file
e.preventDefault();
elements.current.clear();
setIsDraggingFile(false);
}
useEffect(() => {
window.addEventListener("dragenter", enter, { passive: true });
window.addEventListener("dragleave", leave, { passive: true });
window.addEventListener("dragend", end, { passive: true });
window.addEventListener("dragover", over);
window.addEventListener("drop", drop);
return () => {
window.removeEventListener("dragleave", enter);
window.removeEventListener("dragenter", leave);
window.removeEventListener("dragend", end);
window.removeEventListener("dragover", over);
window.removeEventListener("drop", drop);
};
}, []);
return isDraggingFile;
} (This also prevents the default browser behaviour of downloading the file if you miss the drop target.) Usage in components would be just const isDraggingFile = useIsDraggingFile(); I also noticed the rendering of the dragger element is 44 lines of duplicated content, do you think you could deduplicate that into a more easily used component? |
The box and button outline are displayed correctly. However, the default behaviour of prompting a download is not prevented when the target is missed or the host is offline. Also, out of scope for this PR, but we need to add error messages when upload fails because of falid validation or duplicate item. |
This is a browser default - I am not sure if we want to override that? As for host offline, I propose making the icon red or something to show the field not currently being an available target but still showing that it is a drop target I was fairly sure I had seen error messages for file uploads in the past? Might have been in the console or something. |
While it is the default, it is also a bad user experince. Here is what it looks like on Google Drive when you drag a file into the wrong area. One Drive also has the same behaviour. I agree that there should be something to show that upload is possible but not at the current time. I have checked, there is an error message in the console. A 500 response from the server with a message explaining the reason. We shouldn't consider this to be user facing so is still something for us to work on later. |
If microsoft doesn't care about the defaults than I don't either. I am using the default "copy"/"none" icons from the browser - on macos for some reason the "move" and "none" looks the same, not sure how it looks to you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Everything works as expected now. (Firefox, Windows 10)
The arguments for the dropzone component are styled differently to others, but I'm not an expert on react so can't comment on it. Either way it works so I have no issue there.
Added drag and drop overlay to upload button on mods page and save table on instance page.
Also added drag and drop uploading to the mods table for good measure.
Resolves #591