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

feat(upload): add upload button functionality #176

Merged
merged 6 commits into from
Jul 3, 2024

Conversation

safeamiiir
Copy link
Contributor

@safeamiiir safeamiiir commented Jul 1, 2024

Uncomment upload button and let users upload files for experiments

@safeamiiir safeamiiir linked an issue Jul 1, 2024 that may be closed by this pull request
Copy link
Contributor

github-actions bot commented Jul 1, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-sand-0b0e2a903-176.westeurope.3.azurestaticapps.net

Copy link
Contributor

github-actions bot commented Jul 1, 2024

Coverage

Coverage Report
FileStmtsMissCoverMissing
aqueductcore/backend
   context.py34488%49–53, 60–61
   main.py19479%21–24, 31
   session.py13469%24–28
aqueductcore/backend/models
   extensions.py1511491%53, 56, 80–81, 88, 94, 96–97, 143, 177, 219, 247, 253, 261
   orm.py31197%58
aqueductcore/backend/routers
   files.py88298%169–170
   frontend.py241154%22–33, 37
aqueductcore/backend/routers/graphql
   mutations_schema.py58297%48, 135
   query_schema.py45296%66–68
   router.py11191%15
aqueductcore/backend/routers/graphql/mutations
   experiment_mutations.py24196%30
aqueductcore/backend/routers/graphql/resolvers
   experiment_resolver.py25292%28, 36
   tags_resolver.py25388%14, 44, 46
aqueductcore/backend/services
   experiment.py2203285%82, 111–124, 154, 188, 194, 240, 243, 268, 300, 348, 354, 383, 389, 406, 431, 438, 446, 476–482, 489–493, 505–508
   extensions_executor.py88891%39–42, 68, 83, 86, 152–158, 206
   validators.py33294%41, 46
aqueductcore/cli
   commands.py60788%32, 82–85, 94–98, 110–114
   exporter.py49198%71
   importer.py61198%90
TOTAL135110292% 

Tests Skipped Failures Errors Time
101 1 💤 0 ❌ 0 🔥 26.715s ⏱️

@safeamiiir safeamiiir force-pushed the feat/upload-file-button-159 branch from 794a3eb to 37b73a3 Compare July 1, 2024 18:03
Copy link
Contributor

github-actions bot commented Jul 1, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-sand-0b0e2a903-176.westeurope.3.azurestaticapps.net

Copy link
Contributor

github-actions bot commented Jul 1, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-sand-0b0e2a903-176.westeurope.3.azurestaticapps.net

@safeamiiir safeamiiir marked this pull request as draft July 2, 2024 15:53
Copy link
Contributor

github-actions bot commented Jul 2, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-sand-0b0e2a903-176.westeurope.3.azurestaticapps.net

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-sand-0b0e2a903-176.westeurope.3.azurestaticapps.net

@safeamiiir safeamiiir marked this pull request as ready for review July 2, 2024 18:07
Copy link
Contributor

@str-anger str-anger left a comment

Choose a reason for hiding this comment

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

I cannot comment more =)

Comment on lines 124 to 133
if (item.kind === "file") {
const file = item.getAsFile();
if (file) {
handleExperimentFileUpload(file)
}
}
});
} else {
[...ev.dataTransfer.files].forEach((file) => {
handleExperimentFileUpload(file)
Copy link
Contributor

Choose a reason for hiding this comment

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

What this means is "I drop something, if some of these are files, I upload each of them separately", else "if I dropped a collection of files, upload them separately". How may this happen that dropped objects are in one of the collections?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's based to cover older APIs for older chrome versoins or IE, also seemsdataTransfer.files is deprecated.
So I can just not include it.

const { handleExperimentFileUpload } = useFileUpload(experimentUuid)
function handleChangeFile(e: ChangeEvent<HTMLInputElement>) {
if (e.target.files) {
[...e.target.files].forEach(file => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a code duplication

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually not.
This is when using onChange function in <input />
and the one above is for onDrop function when doing Drag and Drop.

Why it's not? Because the Event in these two is different, we need to get the file and then use handleUpload function to upload them.

Copy link
Contributor

@samiralavi samiralavi left a comment

Choose a reason for hiding this comment

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

Thanks @safeamiiir, looks good, waiting to see it in action

Copy link
Contributor

github-actions bot commented Jul 3, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-sand-0b0e2a903-176.westeurope.3.azurestaticapps.net

Copy link
Contributor

github-actions bot commented Jul 3, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-sand-0b0e2a903-176.westeurope.3.azurestaticapps.net

@safeamiiir safeamiiir merged commit 6f5565e into main Jul 3, 2024
26 checks passed
@safeamiiir safeamiiir deleted the feat/upload-file-button-159 branch July 3, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "upload file" button to UI
3 participants