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

React to drag and drop #602

Merged
merged 7 commits into from May 18, 2024

Conversation

Danielv123
Copy link
Member

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

Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.83%. Comparing base (79c1a2d) to head (b70feb3).
Report is 21 commits behind head on master.

Current head b70feb3 differs from pull request most recent head c6abe29

Please upload reports for the commit c6abe29 to get more accurate results.

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.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@Cooldude2606 Cooldude2606 left a 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)

@Hornwitser
Copy link
Member

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?

@Cooldude2606
Copy link
Collaborator

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.

@Danielv123
Copy link
Member Author

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.

@Cooldude2606
Copy link
Collaborator

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.

google drive

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.

@Danielv123
Copy link
Member Author

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?

@Cooldude2606
Copy link
Collaborator

I'm on windows 10 firefox and the copy and none works fine for me. Another issue has arose tho: when the instance state is unknown you can drag onto the area, this causes default behaviour to occur (since we arent stopping the interaction) but the strange part is that it becomes impossible to interact with it a second time until you refresh the page.
img
It could be argued that the correct behaviour is that the first interaction should also be blocked, so this might be an issue that the state is not being set on refresh.

Copy link
Collaborator

@Cooldude2606 Cooldude2606 left a 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.

@Danielv123 Danielv123 merged commit 7489a76 into clusterio:master May 18, 2024
3 checks passed
@Danielv123 Danielv123 deleted the react-to-drag-and-drop branch May 18, 2024 15:57
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.

React to drag and drop
3 participants