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.

Sign up for GitHub

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

Jump to bottom

Feat: Bulk Upload in Asset Manager #29007

Merged
merged 1 commit into from
Jan 30, 2019
Merged

Feat: Bulk Upload in Asset Manager #29007

merged 1 commit into from
Jan 30, 2019

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Jan 18, 2019 •
edited
Loading

Related to #23064.
Closes #28277.

This adds a file picker to the asset manager that allows you to upload multiple images to a workpad without going through an image element. With this, you can load up your workpad with all of the assets you want to use, and easily select them from the asset picker when you add your image elements.

screen shot 2019-01-18 at 10 48 58 am

jan-18-2019 13-11-17

I also added an accept attribute to all the file pickers. Now file pickers in image upload and in the asset manager will only accept images, and the one in the workpad loader will only accept JSON files.

Workpad Loader

Only JSON files are selectable
screen shot 2019-01-18 at 11 06 49 am

Image Upload

Only image files are selectable
screen shot 2019-01-18 at 11 06 10 am

@cqliu1 cqliu1 added review v7.0.0 Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v6.7.0 labels Jan 18, 2019
@cqliu1 cqliu1 requested review from w33ble and ryankeairns January 18, 2019 18:07
@cqliu1 cqliu1 requested a review from a team as a code owner January 18, 2019 18:07
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas

@cqliu1 cqliu1 requested a review from a team as a code owner January 18, 2019 18:07
@elasticmachine
Copy link
Contributor

💔 Build Failed

@w33ble
Copy link
Contributor

w33ble commented Jan 18, 2019

We probably shouldn't hide the asset interface if you don't have anything uploaded anymore.

@w33ble
Copy link
Contributor

w33ble commented Jan 18, 2019 •
edited
Loading

What's up with the close button once you upload things? Can we disable that, or do we need to open an issue in EUI? It seems to be showing how many images I've selected, but it's already uploaded them, so by the time you see it it's not useful anymore.

jan-18-2019 11-56-06

@w33ble
Copy link
Contributor

w33ble commented Jan 18, 2019

Looks like this uploader doesn't de-dupe assets. When I upload the same image from the normal file uploader, it won't create a second asset, it'll reuse the one that's already uploaded:

jan-18-2019 12-02-04

But if I upload the same asset from the assets modal, it'll create a duplicate:

jan-18-2019 12-03-02

w33ble
w33ble suggested changes Jan 18, 2019
View reviewed changes
Copy link
Contributor

@w33ble w33ble left a comment

Choose a reason for hiding this comment

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

Looks great overall, but it needs to de-dupe uploaded assets.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cqliu1
Copy link
Contributor Author

cqliu1 commented Jan 18, 2019 •
edited
Loading

We probably shouldn't hide the asset interface if you don't have anything uploaded anymore.

Good call! I removed the conditional for rendering the asset manager button, and now it'll show up new workpads.

Looks like this uploader doesn't de-dupe assets.

I added the same check we use for image upload to de-dupe the assets in the asset manager.

I also added a loading indicator to disable the filepicker when images are being saved.
jan-18-2019 13-21-19

@cqliu1
Copy link
Contributor Author

cqliu1 commented Jan 18, 2019

What's up with the close button once you upload things? Can we disable that, or do we need to open an issue in EUI? It seems to be showing how many images I've selected, but it's already uploaded them, so by the time you see it it's not useful anymore.

Hmm, I'm not sure what's causing it, but adding the loading indicator should prevent it from happened.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cqliu1
Copy link
Contributor Author

cqliu1 commented Jan 18, 2019

I added an EuiEmptyPrompt for when you have no assets uploaded.

screen shot 2019-01-18 at 2 22 14 pm

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ryankeairns ryankeairns mentioned this pull request Jan 23, 2019
Closed
44 tasks
ryankeairns
ryankeairns requested changes Jan 23, 2019
View reviewed changes
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

The feature itself works as expected, I like it!

There is a little something going on with the modal header markup. Might have to do with how the flex elements are arranged/nested. Compare the Manage Assets header to the Add Element header... see how the FilePicker flows over into the margin:

Manage assets

screenshot 2019-01-23 15 43 18

Add element

screenshot 2019-01-23 15 43 45

w33ble
w33ble approved these changes Jan 25, 2019
View reviewed changes
Copy link
Contributor

@w33ble w33ble left a comment

Choose a reason for hiding this comment

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

This is looking real good!

sweet

@cqliu1
Copy link
Contributor Author

cqliu1 commented Jan 26, 2019

@ryankeairns I added the same style you applied to the file picker in the workpad loader, which makes the height taller, but I can undo that change.

How does this look?
screen shot 2019-01-25 at 5 00 03 pm

@cqliu1 cqliu1 force-pushed the feat/upload-bulk-assets branch from 0e79872 to 12a96b6 Compare January 26, 2019 00:22
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cqliu1
Removed file_upload component
152afa2 
    Moved filepicker style to main.scss. Removed workpad_upload component

    Added filepicker to asset manager

    Removed check for assets in workpad header to always show asset manager button

    Deduped image uploads in asset manager. Added loading indicator to asset manager

    Added empty prompt to display when there are no assets

    Adds additional image upload type checking

    Updated verbiage

    Undid CSS changes to  workpad_loader filepicker
@cqliu1 cqliu1 force-pushed the feat/upload-bulk-assets branch from 12a96b6 to 152afa2 Compare January 30, 2019 17:47
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cqliu1 cqliu1 merged commit cc0e2d0 into elastic:master Jan 30, 2019
@cqliu1 cqliu1 deleted the feat/upload-bulk-assets branch January 30, 2019 20:05
@cqliu1 cqliu1 mentioned this pull request Jan 30, 2019
Merged
cqliu1 added a commit to cqliu1/kibana that referenced this pull request Jan 30, 2019
@cqliu1
Removed file_upload component (elastic#29007)
bbf3c90 
Moved filepicker style to main.scss. Removed workpad_upload component

    Added filepicker to asset manager

    Removed check for assets in workpad header to always show asset manager button

    Deduped image uploads in asset manager. Added loading indicator to asset manager

    Added empty prompt to display when there are no assets

    Adds additional image upload type checking

    Updated verbiage

    Undid CSS changes to  workpad_loader filepicker
cqliu1 added a commit that referenced this pull request Jan 30, 2019
@cqliu1
Removed file_upload component (#29007) (#29654)
5323a57 
[6.x] Feat: Bulk Upload Assets (#29007)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@w33ble w33ble w33ble approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

Assignees
No one assigned
Labels
review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v6.7.0 v7.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cqliu1 @elasticmachine @w33ble @ryankeairns