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

Fix/make file-imput more accessible and use semantic html #97

Merged
merged 8 commits into from
Oct 6, 2024

Conversation

doriengr
Copy link
Contributor

@doriengr doriengr commented Oct 4, 2024

  • Fix bug on delete button
  • Add check if file is really .csv

Close #46

@doriengr doriengr self-assigned this Oct 4, 2024
@doriengr doriengr changed the title Fix/make file input more accessible Fix/make file-imput more accessible and use semantic html Oct 4, 2024
@doriengr doriengr marked this pull request as ready for review October 4, 2024 22:10
@doriengr doriengr added the need-reviewer Send notification in Discord label Oct 5, 2024
@Mohammad96Assaf
Copy link
Contributor

As I mentioned in #98, the reason I originally used a custom style was to solve the localization issue, as I couldn't control the text in the default browser input. I also have another question: what about the progress bar that displays the upload status? Why did you remove it? :)

@doriengr
Copy link
Contributor Author

doriengr commented Oct 5, 2024

As I mentioned in #98, the reason I originally used a custom style was to solve the localization issue, as I couldn't control the text in the default browser input. I also have another question: what about the progress bar that displays the upload status? Why did you remove it? :)

I removed it becauce the selection of a file does never take long and its not needed in my eyes for some couple of seconds. We could implement a process bar while the Import is running.

And the localization works in my eyes

@Mohammad96Assaf
Copy link
Contributor

As I mentioned in #98, the reason I originally used a custom style was to solve the localization issue, as I couldn't control the text in the default browser input. I also have another question: what about the progress bar that displays the upload status? Why did you remove it? :)

I removed it becauce the selection of a file does never take long and its not needed in my eyes for some couple of seconds. We could implement a process bar while the Import is running.

And the localization works in my eyes

The progress bar actually demonstrates the import process. It runs until the server returns a response. Here's the code snippet:

const pr = await axios.post(to, formData, {
    headers: {
        'Authorization': authHeader,
        'Content-Type': 'multipart/form-data',
    }, 
    onUploadProgress: (progressEvent) => {
        if (progressEvent.total) {
            const percentCompleted = Math.round((progressEvent.loaded * 100) / progressEvent.total);
            setProgress(percentCompleted);
        }
    }
});

Sometimes the process might take more than just a few seconds, so in my opinion, the progress bar helps provide feedback to the user. Also, in the backend, the upload process is not yet separated from the import process, which is why the progress bar reflects the entire operation.

@doriengr
Copy link
Contributor Author

doriengr commented Oct 5, 2024

@Mohammad96Assaf is it a solution if we make a new issue for that? As soon as the backend works, we can find a solution for the progress bar that will be shown as soon as the progress in the backend is started?

Copy link
Member

@choffmann choffmann left a comment

Choose a reason for hiding this comment

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

👍

frontend/package.json Show resolved Hide resolved
frontend/src/routes/_protected/settings/import.tsx Outdated Show resolved Hide resolved
frontend/src/routes/_protected/settings/import.tsx Outdated Show resolved Hide resolved
frontend/src/components/general/fileUpload/FileUpload.tsx Outdated Show resolved Hide resolved
@doriengr doriengr merged commit afc3036 into develop Oct 6, 2024
1 check passed
@doriengr doriengr deleted the fix/make-file-input-more-accessible branch October 6, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-reviewer Send notification in Discord
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add import cluster view
3 participants