Skip to content

Fixes error after dropping invalid element over editor area#125943

Merged
sbatten merged 1 commit into
microsoft:mainfrom
jeanp413:fix-124946
Jun 22, 2021
Merged

Fixes error after dropping invalid element over editor area#125943
sbatten merged 1 commit into
microsoft:mainfrom
jeanp413:fix-124946

Conversation

@jeanp413

@jeanp413 jeanp413 commented Jun 10, 2021

Copy link
Copy Markdown
Contributor

This PR fixes #124946

@jeanp413 jeanp413 mentioned this pull request Jun 10, 2021
Closed
@sbatten

sbatten commented Jun 11, 2021

Copy link
Copy Markdown
Member

added reviewers based on those involved in the issue discussion

@isidorn

isidorn commented Jun 22, 2021

Copy link
Copy Markdown
Collaborator

Looks good to me!
@sbatten should we merge this in?

@sbatten sbatten merged commit 5716b11 into microsoft:main Jun 22, 2021
@sbatten sbatten added this to the June 2021 milestone Jun 22, 2021
@jeanp413 jeanp413 deleted the fix-124946 branch June 22, 2021 17:27
Tyriar added a commit that referenced this pull request Jun 24, 2021
@Tyriar
Fix terminal dnd to editor
9403dff 
Drag and drop broken from terminal to editor in #125943 because it no longer
accepted the TERMINAL data transfer type and list views changed to drag text instead of
resources.

Fixes #127077
@Tyriar

Tyriar commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

FYI this broke terminal DND, I'm mostly changing it back in 9403dff

@jeanp413

jeanp413 commented Jun 24, 2021

Copy link
Copy Markdown
Contributor Author

@Tyriar sorry about that, I did this PR before DataTransfers.TERMINALS was added.
But is it really needed to change the list datatype back to DataTransfers.RESOURCES? The point of this PR is to use DataTransfers.RESOURCES just for file resources.
I think that checking for DataTransfers.TERMINALS in editorDropTarge.ts and setting the datatype to DataTransfers.TERMINALS inside the terminal tab list while starting a drag and drop action should fix the terminal DND.

@isidorn

isidorn commented Jun 24, 2021

Copy link
Copy Markdown
Collaborator

@jeanp413 that sounds good to me.
If @Tyriar agrees, maybe you could open a new PR which also tackles the terminal?

@Tyriar

Tyriar commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

Will change 👍

@Tyriar Tyriar mentioned this pull request Jun 24, 2021
Closed
@Tyriar

Tyriar commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

That's tracked in #127077

@isidorn

isidorn commented Jun 25, 2021

Copy link
Copy Markdown
Collaborator

@Tyriar Great. Did you also unrevert your revert?

@Tyriar

Tyriar commented Jun 25, 2021

Copy link
Copy Markdown
Contributor

@isidorn it's like it was in the PR now, just Terminals data transfers are also accepted.

@isidorn

isidorn commented Jun 25, 2021

Copy link
Copy Markdown
Collaborator

Awesome, thanks 👏

@jeanp413 jeanp413 mentioned this pull request Jul 11, 2021
Merged
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@joaomoreno joaomoreno joaomoreno approved these changes

@isidorn isidorn isidorn approved these changes

+1 more reviewer

@bpasero bpasero bpasero approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@sbatten sbatten

Labels

None yet

Projects

None yet

Milestone

June 2021

Development

Successfully merging this pull request may close these issues.

Error after dropping invalid element over editor area

6 participants

@jeanp413 @sbatten @isidorn @Tyriar @joaomoreno @bpasero