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

[Frontend] Data Upload v1: Move Uploaded Files to New Settings Page #37

Merged
merged 7 commits into from Sep 26, 2022

Conversation

mxosman
Copy link
Contributor

@mxosman mxosman commented Sep 23, 2022

Description of the change

Moves the UploadedFiles component (which displays a list of files the user uploaded) to the new Settings page, and updates the Settings page to the new design.

Demo:

Screen.Recording.2022-09-23.at.10.28.18.AM.mov

Loading & Error States (ignore the buttons - updated in the demo above):
Screen Shot 2022-09-22 at 8 59 37 PM
Screen Shot 2022-09-22 at 8 57 35 PM

Related issues

Closes #38

Checklists

Development

This box MUST be checked by the submitter prior to merging:

  • Double- and triple-checked that there is no Personally Identifiable Information (PII) being mistakenly added in this pull request

These boxes should be checked by the submitter prior to merging:

  • Tests have been written to cover the code changed/added as part of this pull request

Code review

These boxes should be checked by reviewers prior to merging:

  • This pull request has a descriptive title and information useful to a reviewer
  • This pull request has been moved out of a Draft state, has no "Work In Progress" label, and has assigned reviewers
  • Potential security implications or infrastructural changes have been considered, if relevant

@mxosman mxosman changed the title [WIP][Frontend] Data Upload v1: Move Uploaded Files to New Settings Page [Frontend] Data Upload v1: Move Uploaded Files to New Settings Page Sep 23, 2022
@mxosman mxosman marked this pull request as ready for review September 23, 2022 16:00
@mxosman mxosman requested a review from a team September 23, 2022 16:00
Copy link
Collaborator

@lilidworkin lilidworkin left a comment

Choose a reason for hiding this comment

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

@mxosman this looks great to me!

One idea for a followup PR (definitely don't need to / want to make this change in this PR since it's unrelated) would it makes sense to get rid of the Save button on this page and just save changes onBlur? It seems like that's probably what Juan had in mind since the new mock doesn't have a Save button. Would that be a hard change to make? Not worth it if so.

@mxosman
Copy link
Contributor Author

mxosman commented Sep 26, 2022

@mxosman this looks great to me!

One idea for a followup PR (definitely don't need to / want to make this change in this PR since it's unrelated) would it makes sense to get rid of the Save button on this page and just save changes onBlur? It seems like that's probably what Juan had in mind since the new mock doesn't have a Save button. Would that be a hard change to make? Not worth it if so.

Thank you, Lili! Absolutely! I didn't think it was worth it last week because I thought we were more time-crunched - but it wouldn't be hard to make the change at all. Will give it a go in a follow up PR! Thanks for suggesting!

@mxosman mxosman merged commit df608e0 into main Sep 26, 2022
@mxosman mxosman deleted the mahmoud/settings-upload branch September 26, 2022 19:42
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.

[Frontend] Move Uploaded Files component to new Settings page
2 participants