SWC-4693 #5612
SWC-4693 #5612
Conversation
- Update UploadDialogWidgetV2 to configure drag-and-drop on render, and to completely re-render when the modal is closed. EntityActionControllerImpl - Configure the uploader when EntityActionControllerImpl is configured to prepare drag-and-drop - Use a single instance of UploadDialogWidgetV2 to ensure the same component instance used to configure drag-and-drop is used when the modal is opened. - Update Drag-and-Drop initialization code to use JsInterop instead of JSNI - Remove feature flag checks for UploadDialogWidgetV2 - Delete UploadDialogWidget (v1)
| // If enabled, uses the v2 uploader (react implementation) for file entity uploads. | ||
| UPLOADER_V2("UPLOADER_V2"), | ||
|
|
There was a problem hiding this comment.
This has been enabled for weeks, let's remove it!
| @@ -483,61 +487,61 @@ public void initializeToastContainer() { | |||
| public void initializeDropZone() { | |||
There was a problem hiding this comment.
Refactored to JsInterop for easier debugging
| dropZone.addEventListener("dragend", e -> hideDropZone.run()); | ||
| dropZone.addEventListener("dragleave", e -> hideDropZone.run()); |
There was a problem hiding this comment.
Add dragend/dragleave handlers to fix bug where if you dragged into the window, then dragged out of the window, the dropzone wouldn't go away.
| @JsNullable | ||
| public String key; | ||
|
|
There was a problem hiding this comment.
All React components can take a key prop
| @JsNullable | ||
| public Callback onUploadReady; |
There was a problem hiding this comment.
Use new callback prop
...java/org/sagebionetworks/web/client/widget/entity/controller/EntityActionControllerImpl.java
Outdated
Show resolved
Hide resolved
| private UploadDialogWidgetV2 getUploadDialogWidget() { | ||
| if (uploadDialogWidgetV2 == null) { | ||
| uploadDialogWidgetV2 = ginInjector.getUploadDialogWidget(); | ||
| view.setUploadDialogWidget(uploadDialogWidgetV2.asWidget()); | ||
| } |
There was a problem hiding this comment.
Use a single instance of the widget to prevent bugs where the widget instance that was configured for drag-and-drop was not the same instance of the widget that is shown to the user. I don't think the present code would have this bug, but I noticed it in an intermediate iteration of this feature and it would otherwise be very easy to reintroduce in a later refactor
There was a problem hiding this comment.
We should do some extra testing with component initialization with this change. After uploading a file (or set of files), if you open the uploader to upload another set of files is the component state properly re-initialized?
There was a problem hiding this comment.
(assume so with the call to configure())
| private boolean canUploadNewFileVersion() { | ||
| return ( | ||
| entityBundle.getEntity() instanceof FileEntity && | ||
| permissions.getCanCertifiedUserEdit() | ||
| ); | ||
| } | ||
|
|
||
| private void configureFileUpload() { | ||
| if (entityBundle.getEntity() instanceof FileEntity) { | ||
| actionMenu.setActionVisible( | ||
| Action.UPLOAD_NEW_FILE, | ||
| permissions.getCanCertifiedUserEdit() | ||
| ); | ||
| actionMenu.setActionListener(Action.UPLOAD_NEW_FILE, this); | ||
| } else { | ||
| actionMenu.setActionVisible(Action.UPLOAD_NEW_FILE, false); | ||
| } | ||
| actionMenu.setActionVisible( | ||
| Action.UPLOAD_NEW_FILE, | ||
| canUploadNewFileVersion() | ||
| ); | ||
| actionMenu.setActionListener(Action.UPLOAD_NEW_FILE, this); | ||
| } | ||
|
|
||
| private boolean canUploadFileToContainer() { | ||
| return ( | ||
| isContainerOnFilesTab(entityBundle.getEntity(), currentArea) && | ||
| permissions.getCanCertifiedUserEdit() | ||
| ); | ||
| } | ||
|
|
||
| private void configureUploadNewFileEntity() { | ||
| if (isContainerOnFilesTab(entityBundle.getEntity(), currentArea)) { | ||
| actionMenu.setActionVisible( | ||
| Action.UPLOAD_FILE, | ||
| permissions.getCanCertifiedUserEdit() | ||
| ); | ||
| actionMenu.setActionListener(Action.UPLOAD_FILE, this); | ||
| } else { | ||
| actionMenu.setActionVisible(Action.UPLOAD_FILE, false); | ||
| } | ||
| actionMenu.setActionVisible(Action.UPLOAD_FILE, canUploadFileToContainer()); | ||
| actionMenu.setActionListener(Action.UPLOAD_FILE, this); |
There was a problem hiding this comment.
Refactor to share logic for these checks
| case UPLOAD_NEW_FILE: | ||
| case UPLOAD_FILE: | ||
| onUploadFile(); |
There was a problem hiding this comment.
These were different at one point, but today they both open the same widget with identical arguments.
| DomGlobal.console.error( | ||
| "EntityUploadHandle ref is null, aborting handleDrop" | ||
| ); |
There was a problem hiding this comment.
This should never happen, and user can recover by just opening the uploader, so I decided not to spend time on more sophisticated error handling.
| private void onUploadReady() { | ||
| globalApplicationState.setDropZoneHandler(this::handleDrop); | ||
| } |
There was a problem hiding this comment.
Defer setting up drag-and-drop until the component tells us it's ready via callback
| private void onClose() { | ||
| eventBus.fireEvent(new EntityUpdatedEvent(entityId)); | ||
| // Re-render the component with a new key to reset the state so it no longer shows the previous set of uploads | ||
| keyCounter++; |
There was a problem hiding this comment.
Ah, there's the re-init. Ignore my previous comment :D
EntityActionControllerImpl
Configure the uploader when EntityActionControllerImpl is configured to prepare drag-and-drop
Use a single instance of UploadDialogWidgetV2 to ensure the same component instance used to configure drag-and-drop is used when the modal is opened.
Update Drag-and-Drop initialization code to use JsInterop instead of JSNI
Remove feature flag checks for UploadDialogWidgetV2
Delete UploadDialogWidget (v1)