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

[DataGrid] Focus gets reset to GridActionCellItem when clicking TextField inside Dialog #3158

Closed
2 tasks done
marcopixel opened this issue Nov 11, 2021 · 10 comments
Closed
2 tasks done
Labels
bug 🐛 Something doesn't work component: data grid 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

@marcopixel
Copy link

marcopixel commented Nov 11, 2021

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

If you open a dialog via <GridActionsCellItem onClick={}> and then click on one of the <TextField> components inside the dialog window, you will loose focus on the first click and after that it works as expected.

2021-11-11.11-18-44.mp4

Expected behavior 🤔

It should focus on the <TextField> component instead to allow for text input.

Steps to reproduce 🕹

Repro: https://codesandbox.io/s/modest-borg-6i0r2?file=/src/ArticleListDialog.js
Steps:

  1. Click on the (+) icon on one of the row items
  2. Click on a random <TextField> inside the dialog window

Context 🔦

I want to execute actions based on the items inside the DataGrid and i need to fill out additional data like date, email or domain name.

This seems to be related to #1295, as it also tries to focus the DataGridCell behind it (you can see the focus when closing the window).

Your environment 🌎

`npx @mui/envinfo`
  Browsers used: Chrome: 95.0.4638.69
  
  System:
    OS: Windows 10 10.0.19042
    CPU: (8) x64 Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
    Memory: 6.30 GB / 15.79 GB
  Binaries:
    Node: 14.17.4 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.17 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.14.14 - C:\Program Files\nodejs\npm.CMD
  Managers:
    pip3: 21.1.3 - C:\Python39\Scripts\pip3.EXE
  Utilities:
    Git: 2.32.0. - C:\Users\vockner\AppData\Local\Programs\Git\cmd\git.EXE
  SDKs:
    Windows SDK:
      AllowAllTrustedApps: Disabled
      Versions: 10.0.17763.0
  IDEs:
    VSCode: 1.62.1 - C:\Users\vockner\AppData\Local\Programs\Microsoft VS Code\bin\code.CMD
  Languages:
    Python: 3.9.6 - C:\Python39\python.EXE
  Browsers:
    Chrome: 95.0.4638.69
    Edge: Spartan (44.19041.1266.0), Chromium (95.0.1020.44)
    Internet Explorer: 11.0.19041.1202

Order ID 💳 (optional)

No response

@marcopixel marcopixel added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 11, 2021
@m4theushw
Copy link
Member

This is a side effect of #2862. Since there's a event.stopPropagation(), when the button is clicked the focus management can't detect that the focus was lost (it listens for any click in the document) because the click is never propagated. Initially I proposed to listen for the click event in the capture phase to avoid this kind of issue, however this is aggressive as users would not be able to cancel this behavior if they want. The following diff fixes the bug by removing the event.stopPropagation() while still blocking the row from being selected during the click. Do you want to submit a PR?

As a workaround, removing event.stopPropagation() from your code also fixes it. However, the row will appear selected when opening the dialog.

diff --git a/packages/grid/_modules_/grid/components/cell/GridActionsCellItem.tsx b/packages/grid/_modules_/grid/components/cell/GridActionsCellItem.tsx
index 98c39c04e..62e7ac0bb 100644
--- a/packages/grid/_modules_/grid/components/cell/GridActionsCellItem.tsx
+++ b/packages/grid/_modules_/grid/components/cell/GridActionsCellItem.tsx
@@ -13,26 +13,18 @@ export type GridActionsCellItemProps = {
 );
 
 const GridActionsCellItem = (props: GridActionsCellItemProps) => {
-  const { label, icon, showInMenu, onClick, ...other } = props;
-
-  const handleClick = (event: any) => {
-    event.stopPropagation();
-
-    if (onClick) {
-      onClick(event);
-    }
-  };
+  const { label, icon, showInMenu, ...other } = props;
 
   if (!showInMenu) {
     return (
-      <IconButton size="small" aria-label={label} {...(other as any)} onClick={handleClick}>
+      <IconButton size="small" aria-label={label} {...(other as any)}>
         {React.cloneElement(icon!, { fontSize: 'small' })}
       </IconButton>
     );
   }
 
   return (
-    <MenuItem {...(other as any)} onClick={onClick}>
+    <MenuItem {...(other as any)}>
       {icon && <ListItemIcon>{icon}</ListItemIcon>}
       {label}
     </MenuItem>
diff --git a/packages/grid/_modules_/grid/hooks/features/selection/useGridSelection.ts b/packages/grid/_modules_/grid/hooks/features/selection/useGridSelection.ts
index d427678f1..2a7e235c9 100644
--- a/packages/grid/_modules_/grid/hooks/features/selection/useGridSelection.ts
+++ b/packages/grid/_modules_/grid/hooks/features/selection/useGridSelection.ts
@@ -64,6 +64,7 @@ export const useGridSelection = (
   >,
 ): void => {
   const logger = useGridLogger(apiRef, 'useGridSelection');
+  const lastClickedCell = React.useRef<GridCellParams | null>(null);
 
   const propSelectionModel = React.useMemo(() => {
     if (props.selectionModel == null) {
@@ -263,6 +264,14 @@ export const useGridSelection = (
         return;
       }
 
+      const { current: clickedCellParams } = lastClickedCell;
+      lastClickedCell.current = null;
+
+      // Ignore if before the row click a cell of `actions` type was clicked
+      if (clickedCellParams?.colDef.type === 'actions') {
+        return;
+      }
+
       const hasCtrlKey = event.metaKey || event.ctrlKey;
 
       if (event.shiftKey && (canHaveMultipleSelection || checkboxSelection)) {
@@ -292,6 +301,10 @@ export const useGridSelection = (
     ],
   );
 
+  const handleCellClick = React.useCallback((params: GridCellParams) => {
+    lastClickedCell.current = params;
+  }, []);
+
   const preventSelectionOnShift = React.useCallback(
     (params: GridCellParams, event: React.MouseEvent) => {
       if (canHaveMultipleSelection && event.shiftKey) {
@@ -328,6 +341,7 @@ export const useGridSelection = (
 
   useGridApiEventHandler(apiRef, GridEvents.visibleRowsSet, removeOutdatedSelection);
   useGridApiEventHandler(apiRef, GridEvents.rowClick, handleRowClick);
+  useGridApiEventHandler(apiRef, GridEvents.cellClick, handleCellClick);
   useGridApiEventHandler(
     apiRef,
     GridEvents.rowSelectionCheckboxChange,

@m4theushw m4theushw added bug 🐛 Something doesn't work component: data grid 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. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 11, 2021
@marcopixel
Copy link
Author

marcopixel commented Nov 14, 2021

Thanks for the quick response and explanation @m4theushw :)

I would appreciate it if you guys could add the fix in one of the pull requests (like #2725) as I lack the time to properly contribute to this issue. 👍 Also simply removing event.stopPropagation() did not fix it for me, but since this isn't breaking right now i'm not too worried about a timeframe for a fix.

@flaviendelangle
Copy link
Member

@m4theushw your code only works if clicking on the whole cell disables the selection.
For the Tree Data grouping cell, I only disable the selection on click when clicking on the toggle icon.

@michael-land
Copy link

I'm also facing this issue @v5.2.0

@kodeschooler
Copy link

I'm also having this issue after I switched my list pages to use the DataGrid. My mui version is 5.1.1. I have no idea what to do about it, and it's pretty glaring to the user. You click in the text box within the dialog and it shifts focus back to the grid, then you click the text field again and it removes focus from grid back to text field and the focus stays in the text field. Note- if I put a button above the grid and trigger the dialog that way there's no problem, it only happens when they click the edit icon in the row and trigger the dialog that way. I'm using getActions: (params: GridRowParams) and rendering a GridActionCellItem with EditIcon like in the examples:

getActions: (params: GridRowParams) => [ <GridActionsCellItem icon={<EditIcon sx={{ color: "primary.dark" }} />} onClick={() => handleEditRowClick(params.id.toString())} label="Edit" />, ],

Is there a work around I can use in my code, even if it's not ideal? I don't want to check this in like this. Thank you!

@marcopixel
Copy link
Author

marcopixel commented Jan 14, 2022

I've found a workaround today for this issue by simply adding onClick={(e) => e.stopPropagation()} to the TextField.
Haven't found any issues so far and seems to be easy enough to implement in the meanwhile until this problem is solved.

Example: https://codesandbox.io/s/zealous-moon-equgn?file=/src/ArticleListDialog.js:1905-1941

<TextField
    key={index}
    id={field.id}
    label={field.label}
    type={field.type}
    name={field.id}
    required
    value={values[field.id]}
    margin="normal"
    variant="standard"
    fullWidth
    InputLabelProps={{
      shrink: true
    }}
+   onClick={(e) => e.stopPropagation()}
    onChange={(e) => {
      e.stopPropagation();
      handleInputValue(e);
    }}
/>

@kodeschooler
Copy link

kodeschooler commented Jan 14, 2022

@marcopixel that worked for me, thank you so much!! Saved my day.

if you later encounter any consequence of making that change please post it here, as will I.

@yannkost
Copy link

yannkost commented Feb 4, 2022

I'm also having trouble using forms in the Dialog due to this issue.
The fix from @marcopixel made my day!

@kodeschooler
Copy link

Never mind, I upgraded to grid v5.6.1 and now the issue is fixed! Thanks !!

@m4theushw
Copy link
Member

I'm closing since the issue has already been fixed and released, probably by #3929.

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: data grid 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

No branches or pull requests

6 participants