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

Refactor: upload code #2551

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Refactor: upload code #2551

wants to merge 7 commits into from

Conversation

albullington
Copy link
Collaborator

Creative day project. I bailed on background uploading and settled for refactoring in an attempt to make our upload code make slightly more sense. A few things in here:

  • Use a single useEffect in useUploadObservations to make it clear when and why uploads are starting
  • Refactor toolbar progress and individual progress to be derived any time upload progress is incremented, and simplify calculations
  • Use a standardized startIndividualUpload function that works across the multi upload flow and MyObs
  • Break up all functionality in uploadObservation into bite-sized functions with clear naming
  • Create addObservationsToUploadQueue function to set upload state (adding to queue, prepping the progress status) in a single function
  • Remove eventListeners to listen for progress updates and instead pass this into uploadObservation via the updateTotalUploadProgress callback, which makes it easy to see exactly when and why we're incrementing progress in a single file
  • Break the toolbar timeout into a separate hook from the rest of the upload code, useToolbarTimeout, since it has a single purpose to update state & doesn't have to do with the rest of the upload queue / progress

Would be good to get a review on this since I'm sure I broke something / missed something. Hopefully this makes things a little less complicated.

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.

1 participant