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

fix(playground): ignore keyboard events in template messages for drag and drop #4945

Merged
merged 7 commits into from
Oct 10, 2024

Conversation

Parker-Stafford
Copy link
Contributor

@Parker-Stafford Parker-Stafford commented Oct 10, 2024

Fixes bug where typing a space or enter on while in the text field caused the keyboard event to get swallowed by dnd-kit and switch to drag mode.
Fixed by migrating to dnd-kit non experimental

Typing / mouse and keyboard drag and drop

Screen.Recording.2024-10-10.at.3.55.28.PM.mov

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 10, 2024
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Oct 10, 2024
@@ -32,22 +55,31 @@ export function PlaygroundChatTemplate(props: PlaygroundChatTemplateProps) {
throw new Error(`Invalid template type ${template.__type}`);
}

const sensors = useSensors(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is not necessary but does make the keyboard sorting much much better, can remove if we don't care

@@ -5,7 +5,8 @@ export type GenAIOperationType = "chat" | "text_completion";

let playgroundInstanceIdIndex = 0;
let playgroundRunIdIndex = 0;
let playgroundMessageIdIndex = 0;
// This value must be truthy in order for message re-ordering to work
let playgroundMessageIdIndex = 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't bottom this out but an item with id of 0 is not draggable, assume there is some if(id) check going on somewhere

Copy link
Contributor

Choose a reason for hiding this comment

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

oh insteresting

Comment on lines +75 to +77
if (!over || active.id === over.id) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

can you help me understand this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if the dragged element not over any other element, or it's over itself, no need to reorder the list so we cut out early, pulled this from an example, probably works without it, just an optimization i suppose

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha

Copy link
Contributor

@mikeldking mikeldking 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 this. Very nice

@Parker-Stafford Parker-Stafford merged commit a6a36de into playground Oct 10, 2024
7 checks passed
@Parker-Stafford Parker-Stafford deleted the parker/fix-dnd-keyboard branch October 10, 2024 23:39
mikeldking pushed a commit that referenced this pull request Oct 11, 2024
… and drop (#4945)

* fix(playground): ignore keyboard events in template messages for drag and drop

* pull default pointer sensor from dnd-kit/dom

* update file name casing via git

* migrate to non-experimental dnd-kit

* remove unused experimental code, update naming and remove unnecessary props

* remove strategy

* fix forward ref warning
Parker-Stafford added a commit that referenced this pull request Oct 11, 2024
… and drop (#4945)

* fix(playground): ignore keyboard events in template messages for drag and drop

* pull default pointer sensor from dnd-kit/dom

* update file name casing via git

* migrate to non-experimental dnd-kit

* remove unused experimental code, update naming and remove unnecessary props

* remove strategy

* fix forward ref warning
mikeldking pushed a commit that referenced this pull request Oct 11, 2024
… and drop (#4945)

* fix(playground): ignore keyboard events in template messages for drag and drop

* pull default pointer sensor from dnd-kit/dom

* update file name casing via git

* migrate to non-experimental dnd-kit

* remove unused experimental code, update naming and remove unnecessary props

* remove strategy

* fix forward ref warning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants