[data grid] Consider making new option for disable cell focus outline

[zzossig]

I don't need the outline when cell focused. I couldn't find an easy way to remove the outline.

• [ x] I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

I expect to use the option like this.

<XGrid 
  rows={...} 
  cols={...}
  disableCellFocusOutline
...
/>

Motivation 🔦

In my project, x-grid is used not only for data, but also for community board. The community board just need some basic feature of the x-grid and it looks awkward with the outline.

Order id 💳

#27772

[flaviendelangle]

I find it weird to be able to focus a cell but to have no visual feedback on it
If we added an option to disable the cell focus / navigation would it work for you ?

[zzossig]

Yes.
I think no-visual-feedback could be useful sometimes cause the product can be used variety of purposes.
Also, I think it is a good idea for users to give them choose options. It is better than nothing they can do with it.
Right now, I just used CSS to remove the cell-focused-outline but behind the scene, cells are still selectable.
It would improve performance if it can be disabled too.

[m4theushw]

Did you apply none to both :focus-within and :focus selectors? I'm asking because by doing so the outline becomes completely invisible and I don't think we need a prop in this case.

const useStyles = makeStyles({
  root: {
    "& .MuiDataGrid-cell:focus-within, & .MuiDataGrid-cell:focus": {
      outline: "none"
    }
  }
});

However, if you're trying to create a "read-only" grid, without any interaction, then #2113 might be what you're looking for. In this case, it requires more work than only removing the outline.

[zzossig]

I'm asking because by doing so the outline becomes completely invisible and I don't think we need a prop in this case.

Yes, by doing so the outline becomes invisible but I think we need a prop.
I have tested that when I type up, down key in XGrid, scroll moves.
It means XGrid still calculate a tabIndex behind the scenes.
But I think XGrid doesn't need to do so since I made the outline invisible.
I'm not saying read-only mode. I do need interactions like filtering or sorting.

So, this is the point to introduce the new prop

• It is easy more than using css
• cell outline invisible when selected
• no tabIndex calculation
• but still can do interactions like filtering or sorting

@m4theushw Do you think this is overkill?

[m4theushw]

Do you think this is overkill?

No, in this case it makes sense. What I was trying to say ealier is that if the requirement is to only remove the outline, keeping the everything as it is, then CSS can be used. But since you raised the need to also disable cell navigation and the tabIndex calculation, then we could provide a prop to address it properly.

Just a quick heads up. You don't need to use !important if you're using makeStyles: #2429 (comment)

[zzossig]

if the requirement is to only remove the outline, keeping the everything as it is, then CSS can be used

In my opinion, it is still worth considering to make the new option even if only requirement is to remove the outline.
Every time people want to remove the outline, they have to search for a class name.
You are already familiar with the XGrid and feel like it is very easy to use CSS to remove it but it is not.
I had to struggle to find out which class is impacted to header cell outline when focused.
No one told me(or couldn't find document), so I had to find the class name in chrome dev tools.
So, think about other people who want to remove the cell-focused-outline.

This is the steps they might experience

• look for a disable option
• if they can't found, search how to remove it
• they found a way using css.
• they also want to remove the header cell focused outline(at least in my case).
• find using chrome dev tools which class name is bound to it

This is why I said

It is easy more than using css

[flaviendelangle]

I'm in favor of thinking of a new prop, even if it's not implemented straight away.
@m4theushw what would you think should be the scope of this new prop ?

• disableKeyboardNavigation ? =>
• tabIndex={-1} ?

We also have to make sure that the cell and row edition modes are either forbidden when this prop is provided OR have a correct behavior.

[m4theushw]

@flaviendelangle Currently, setting the tabIndex doesn't do anything because props are not spread to the root (we should allow that, like in the core). Disabling the keyboard navigation is not a simple task, there's the easy way and the right way. The easy way is disabling the handler of the cellNavigationKeyDown and columnHeaderNavigationKeyDown events. But both events would still be published, which is not ideal. To not publish the event, several hooks have to be modified, or we remove these events and use only the cellKeyDown event to trigger the navigation. In theory, these events should only be published when a navigation key is pressed, however, they are fired for non-navigation keys too;

https://github.com/mui-org/material-ui-x/blob/053e1f24ed347a4b3090d7f06ee2ce1698b065c3/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts#L335-L336

We also have to make sure that the cell and row edition modes are either forbidden when this prop is provided OR have a correct behavior.

In #2113, the user still wants to edit cells.

[montanaflynn]

I would also like this to be a prop option.

I want to select rows, not individual cells. The individual cell focus is distracting in my case.

[wahajStar]

Did you apply none to both :focus-within and :focus selectors? I'm asking because by doing so the outline becomes completely invisible and I don't think we need a prop in this case.

const useStyles = makeStyles({
  root: {
    "& .MuiDataGrid-cell:focus-within, & .MuiDataGrid-cell:focus": {
      outline: "none"
    }
  }
});

However, if you're trying to create a "read-only" grid, without any interaction, then #2113 might be what you're looking for. In this case, it requires more work than only removing the outline.

Can u please provide an example... how can we do with sx props.

[joserodolfofreitas]

@wahajStar and others coming for example with sx props:

// you can import gridClasses from the data grid packages

    sx={{
          [`& .${gridClasses.cell}:focus, & .${gridClasses.cell}:focus-within`]: {
            outline: 'none',
          },
          [`& .${gridClasses.columnHeader}:focus, & .${gridClasses.columnHeader}:focus-within`]:
            {
              outline: 'none',
            },
        }}

live example

Maybe this could become a recipe until we have the prop, @cherniavskii.

[ortonomy]

I would also like this to be a prop option.

I want to select rows, not individual cells. The individual cell focus is distracting in my case.

This is what I want/came looking for. I don't want a spread sheet with cells, I just want rows.

[theotonge]

Adding another vote for a prop - In some cases you only want to select rows.

[Ririshi]

Disabling focus tracking altogether (with a prop) would reduce state changes in a GridStateInitialization hook (hook 30 in my React profiler), reducing re-renders. In my development server, a focus state update triggers a render that takes between 100 and 250 ms. I can't exactly measure how that translates to production builds (it's not nearly as disruptive or noticeable compared to dev builds), since profiling doesn't work there, but eliminating those extra renders would be nice regardless.

In my case, interaction with the grid would still be possible because of on-click handlers. Perhaps this use-case is too niche to care about, but I personally really don't need focus tracking. Keeping the focus state null (rather than the suggested workaround by removing the styling) is preferable for my use-case.

[wahajStar]

@wahajStar and others coming for example with sx props:

// you can import gridClasses from the data grid packages

    sx={{
          [`& .${gridClasses.cell}:focus, & .${gridClasses.cell}:focus-within`]: {
            outline: 'none',
          },
          [`& .${gridClasses.columnHeader}:focus, & .${gridClasses.columnHeader}:focus-within`]:
            {
              outline: 'none',
            },
        }}

live example

Maybe this could become a recipe until we have the prop, @cherniavskii.

@joserodolfofreitas
Not working with DataGridPro can u please help...

[joserodolfofreitas]

@wahajStar,
Here's a live example using the DataGridPro.
No outline demo - DataGridPro

[doberkofler]

When disabling the cell focus as suggested, the grid does no longer shoe the cell focus but only selects the complete row, but this selection no longer moves when using the keyboard navigation.

[rajkumar4041]

Did you apply none to both :focus-within and :focus selectors? I'm asking because by doing so the outline becomes completely invisible and I don't think we need a prop in this case.

const useStyles = makeStyles({
  root: {
    "& .MuiDataGrid-cell:focus-within, & .MuiDataGrid-cell:focus": {
      outline: "none"
    }
  }
});

However, if you're trying to create a "read-only" grid, without any interaction, then #2113 might be what you're looking for. In this case, it requires more work than only removing the outline.

it works but the border of mui datagrid header is still shown

[wamae-ndiritu]

sx={{
            "& .MuiDataGrid-cell:focus, & .MuiDataGrid-cell:focus-within": {
              outline: "none",
            },
          }}

I found this to work for me.

[aniketkudale]

Ideally this should be handled via prop.