Skip to content

fix(createDropzone): add support for dragenter event on drag#216

Open
mitogh wants to merge 5 commits intoIBM:mainfrom
mitogh:fix/add-support-dragenter-event
Open

fix(createDropzone): add support for dragenter event on drag#216
mitogh wants to merge 5 commits intoIBM:mainfrom
mitogh:fix/add-support-dragenter-event

Conversation

@mitogh
Copy link

@mitogh mitogh commented Mar 12, 2026

This event can be used by callers to set up UI with classes or animations when the drag starts, and aligns with Aspera Connect, allowing callers to listen for drag enter event.

This event can be used by callers to setup UI with classes or animations
when the drag starts.
@mitogh mitogh requested a review from a team as a code owner March 12, 2026 19:46
Copy link
Member

@dwosk dwosk left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @mitogh! Some minor comments, let me know what you think.

element.addEventListener('dragover', dragEvent);
element.addEventListener('drop', dropEvent);
asperaSdk.globals.dropZonesCreated.set(elementSelector, [{event: 'dragover', callback: dragEvent}, {event: 'drop', callback: dropEvent}]);
registeredListeners.forEach(({event, callback: listener}) => {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than register listeners by default for each drag drop event type, we can leverage the connectOptions argument, and default to false. I think this would be closer to how it worked in the Connect SDK. Additional events would be an opt-in behavior.

For example:

if (connectOptions?.dragEnter) {
  // Register `dragenter` event listener...
}

Copy link
Author

Choose a reason for hiding this comment

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

Thanks mostly I was wondering if connectOptions was still expected to be respected looks like that's the case I can add the logic for all connectOptions options moving forward.

Copy link
Author

Choose a reason for hiding this comment

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

Also, due to the name, my initial thinking was this is only something affecting connect, but let me know if we are okay with using those params, and I will update the code accordingly.

src/app/core.ts Outdated

const dragEnterEvent = (event: DragEvent) => {
event.preventDefault();
callback({event, files: null as unknown as DataTransferResponse});
Copy link
Member

Choose a reason for hiding this comment

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

Should we just change DataTransferResponse.files to be optional? Then callers would be forced to check first and only drop event would actually have the files.

};

const registeredListeners: {event: string; callback: (event: any) => void}[] = [
{event: 'dragenter', callback: dragEnterEvent},
Copy link
Member

Choose a reason for hiding this comment

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

Should we also add dragleave? Also not sure if casing matters here.

mitogh added 4 commits March 12, 2026 15:46
This event can be used by callers to setup UI with classes or animations
when the drag starts.

Signed-off-by: Crisoforo Gaspar <3921289+mitogh@users.noreply.github.com>
…spera-sdk-js into fix/add-support-dragenter-event
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.

2 participants