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

[Chip] Backspace and delete keys don't work when nesting an input #20297

Closed
2 tasks done
bunste opened this issue Mar 27, 2020 · 4 comments · Fixed by #20368
Closed
2 tasks done

[Chip] Backspace and delete keys don't work when nesting an input #20297

bunste opened this issue Mar 27, 2020 · 4 comments · Fixed by #20368
Labels
bug 🐛 Something doesn't work component: chip This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@bunste
Copy link

bunste commented Mar 27, 2020

I'm using a TextField component as label prop for a Chip. That worked without problems for a long time. Since the type of label is specified with node, I assumed that I could insert any child component there. Since I recently updated to the latest version, there is the following problem: I can still add text (cut and paste works too), but I can no longer delete text with the backspace or delete keys.

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Text cannot be deleted using backspace / delete.

Expected Behavior 🤔

Text can be deleted using backspace / delete.

Steps to Reproduce 🕹

https://codesandbox.io/s/sleepy-dust-yjiee

Steps:

  1. Click on the TextField with the current value "foo"
  2. Try to delete the word by pressing the backspace key
  3. Nothing happens...

Context 🔦

I built a editable Chip component. First, the user sees a normal Chip. But when he clicks on it, he can edit the content on-the-fly. As I said at the beginning, it worked very well for a long time.

Your Environment 🌎

Tech Version
Material-UI v4.9.7
React v16.13.0
Browser Google Chrome Version 70.0.3538.77 (Official Build) (64-bit)
@bunste
Copy link
Author

bunste commented Mar 27, 2020

I think it has to do with some of the recent changes to the Chip component (see this commit).

handleKeyDown will do event.preventDefault() for backspace and delete keyboard events.

I wonder if my use case is no longer supported or if this is a bug that should be fixed?

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: chip This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. labels Mar 30, 2020
@oliviertassinari oliviertassinari changed the title Backspace and delete keys don't work when using a TextField as the 'label' of a Chip [Chip] Backspace and delete keys don't work when nesting an input Mar 30, 2020
@oliviertassinari
Copy link
Member

@bunste Thanks for the report, I think that we should have the handler of keyDown and keyUp to use the same logic. What do you think of this patch to align them?

diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js
index f80d27599..09abc369d 100644
--- a/packages/material-ui/src/Chip/Chip.js
+++ b/packages/material-ui/src/Chip/Chip.js
@@ -260,6 +260,10 @@ export const styles = (theme) => {
   };
 };

+function isDeleteKeyboardEvent(keyboardEvent) {
+  return keyboardEvent.key === 'Backspace' || keyboardEvent.key === 'Delete';
+}
+
 /**
  * Chips represent complex entities in small blocks, such as a contact.
  */
@@ -295,19 +299,21 @@ const Chip = React.forwardRef(function Chip(props, ref) {
     }
   };

-  const isDeleteKeyboardEvent = (keyboardEvent) =>
-    keyboardEvent.key === 'Backspace' || keyboardEvent.key === 'Delete';
-
   const handleKeyDown = (event) => {
+    if (onKeyDown) {
+      onKeyDown(event);
+    }
+
+    // Ignore events from children of `Chip`.
+    if (event.currentTarget !== event.target) {
+      return;
+    }
+
     if (isDeleteKeyboardEvent(event)) {
       // will be handled in keyUp, otherwise some browsers
       // might init navigation
       event.preventDefault();
     }
-
-    if (onKeyDown) {
-      onKeyDown(event);
-    }
   };

   const handleKeyUp = (event) => {

Do you want to work on a pull request? :)

@chaudharykiran
Copy link
Contributor

I can work on the this PR? cc @oliviertassinari @bunste

@bunste
Copy link
Author

bunste commented Mar 31, 2020

@chaudharykiran I would also have tried to create a pull request, but you are welcome to do so!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: chip This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants