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

Dashboard: File Upload UI Component #3059

Merged
merged 13 commits into from
Jul 10, 2020
Merged

Conversation

BrittanyIRL
Copy link
Contributor

@BrittanyIRL BrittanyIRL commented Jul 7, 2020

Summary

Settings view in Dashboard needs a simple drag and drop file upload component. Drew on the editor for structure.

Screen Shot 2020-07-08 at 1 10 45 PM

Screen Shot 2020-07-08 at 1 10 52 PM

Screen Shot 2020-07-09 at 11 24 59 AM

Relevant Technical Choices

  • Relies on native event handler functionality for drag and drop and file upload
  • Passes files to parent to process how the parent needs data to avoid locking use cases down
  • Delete functionality is passing the index of the upload as well as the file itself back to parent because of unknowns at the moment.
  • Uses story editor's getResourceFromLocalFile util to try and keep structures in sync behind the surface

To-do

  • Update icon from placeholder x to upload svg - need permissions in figma to download assets
  • Determine 'delete' functionality
  • Follow up on when the file name should be blue like in figma
  • Follow up on if the "You can also drag your logo here" text should only be visible if there's not currently uploads (I think this is the case?)
  • Follow up on delete functionality (this will be after updated designs, read something about the ability to delete or also edit)
  • Get updated button style once PR for dialogs is merged
  • Tests

User-facing changes

  • Changes only exist in storybook

Testing Instructions

  • See storybook Dashboard/Components/FileUpload
  • Upload file(s) with "button" (it's a label) and try drag and dropping, you should see an image show up for each with file name
  • Hover over an uploaded image and click the 'x' to delete it, see actions trigger in storybook.
  • See storybook Dashboard/Views/EditorSettings to see the fileUpload in the settings page. Please note that the dummy functionality isn't hooked up in the view storybook ONLY the components since the view relies on context and since we don't know for sure what all is going to happen here re hooking up the real data I didn't want to spend the time on it.

drop upload
file upload

(placeholder UI for functionality)
placeholder delete


Addresses #2747

@codecov
Copy link

codecov bot commented Jul 7, 2020

Codecov Report

Merging #3059 into master will decrease coverage by 0.04%.
The diff coverage is 79.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3059      +/-   ##
==========================================
- Coverage   81.77%   81.73%   -0.05%     
==========================================
  Files         723      735      +12     
  Lines       12132    12283     +151     
==========================================
+ Hits         9921    10039     +118     
- Misses       2211     2244      +33     
Flag Coverage Δ
#karmatests 63.04% <26.56%> (-0.40%) ⬇️
#unittests 66.53% <78.12%> (+0.04%) ⬆️
Impacted Files Coverage Δ
...rd/app/views/editorSettings/publisherLogo/index.js 22.22% <0.00%> (-44.45%) ⬇️
...c/dashboard/app/views/editorSettings/components.js 60.00% <50.00%> (+2.85%) ⬆️
...ssets/src/dashboard/components/fileUpload/index.js 88.88% <88.88%> (ø)
assets/src/dashboard/components/button/index.js 100.00% <100.00%> (ø)
assets/src/dashboard/constants/components.js 100.00% <100.00%> (ø)
...src/dashboard/app/views/shared/templateGridView.js 81.81% <0.00%> (-7.08%) ⬇️
...ts/src/dashboard/components/pageStructure/index.js 85.36% <0.00%> (-1.82%) ⬇️
assets/src/story-embed-block/block/edit.js 3.07% <0.00%> (-0.05%) ⬇️
includes/Plugin.php 100.00% <0.00%> (ø)
includes/Story_Post_Type.php 95.06% <0.00%> (ø)
... and 23 more

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2020

Size Change: +4.8 kB (0%)

Total Size: 1.06 MB

Filename Size Change
assets/js/edit-story.js 460 kB +950 B (0%)
assets/js/stories-dashboard.js 524 kB +2.49 kB (0%)
assets/js/web-stories-activation-notice.js 55.8 kB +664 B (1%)
assets/js/web-stories-embed-block.js 16.4 kB +697 B (4%)
ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 269 B 0 B
assets/css/stories-dashboard.css 305 B 0 B
assets/css/web-stories-embed-block.css 515 B 0 B

compressed-size-action

@BrittanyIRL BrittanyIRL force-pushed the feature/2747-file-upload branch from c041dbc to 7faed40 Compare July 8, 2020 20:06
@BrittanyIRL BrittanyIRL marked this pull request as ready for review July 8, 2020 20:16
@BrittanyIRL BrittanyIRL added this to the Sprint 32 milestone Jul 8, 2020
@BrittanyIRL BrittanyIRL changed the title [DRAFT] Dashboard: File Upload UI Component Dashboard: File Upload UI Component Jul 8, 2020
@BrittanyIRL BrittanyIRL force-pushed the feature/2747-file-upload branch from 71341a4 to 60b86d0 Compare July 9, 2020 18:27
@carloskelly13
Copy link
Contributor

This works well and I tested the story and the state correctly keeps track of assets as they are added and removed. The only visual glitch I see is the x appears to be pushed up a little bit and perhaps needs to be moved down to be more center. Extra points if it could fade it nicely when hovered.

Screen Shot 2020-07-09 at 3 41 48 PM

@BrittanyIRL BrittanyIRL requested a review from carloskelly13 July 9, 2020 22:26
@BrittanyIRL
Copy link
Contributor Author

This works well and I tested the story and the state correctly keeps track of assets as they are added and removed. The only visual glitch I see is the x appears to be pushed up a little bit and perhaps needs to be moved down to be more center. Extra points if it could fade it nicely when hovered.

Screen Shot 2020-07-09 at 3 41 48 PM

All fixed ~
transition error button

Copy link
Contributor

@mariano-formidable mariano-formidable left a comment

Choose a reason for hiding this comment

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

One super nit, that is tangentially related, I attempted to test the file upload component using the "PublishLogo" story (Views/EditorSettings/PublisherLogo) an e.preventDefault is not a function error when I try and drag/upload a file. It looks like when that story was written it was expecting onUpdatePublisherLogo to pass in an event object, but now it's passing in different thangs and so the story is breaking.

@carloskelly13 carloskelly13 self-requested a review July 9, 2020 22:44
@carloskelly13
Copy link
Contributor

e.preventDefault?.(); should fix that.

@BrittanyIRL
Copy link
Contributor Author

One super nit, that is tangentially related, I attempted to test the file upload component using the "PublishLogo" story (Views/EditorSettings/PublisherLogo) an e.preventDefault is not a function error when I try and drag/upload a file. It looks like when that story was written it was expecting onUpdatePublisherLogo to pass in an event object, but now it's passing in different thangs and so the story is breaking.

ah! so this is actually because i spaced I did that in storybook temporarily before hooking up the fileUpload. the event's preventDefault is taken care of in the fileUpload for consistency - it just needed to be removed - but now we can show the actual props getting passed on upload! Thanks for testing this view!

Copy link
Contributor

@mariano-formidable mariano-formidable left a comment

Choose a reason for hiding this comment

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

Another super nit, but overall looks great to me!

@BrittanyIRL BrittanyIRL merged commit 7be991c into master Jul 10, 2020
@BrittanyIRL BrittanyIRL deleted the feature/2747-file-upload branch July 10, 2020 15:15
@swissspidy swissspidy added the Type: Enhancement New feature or improvement of an existing feature label Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Dashboard Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants