mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.08k stars 1.26k forks source link

[data grid] Column Header tooltip should be displayed on keyboard focus (especially when a column has ellipsis) #13131

Closed shahidify closed 3 months ago

shahidify commented 4 months ago

The problem in depth

I am a MUI Pro license user. I am working on an accessibility feature that needs to display column header tooltip on keyboard focus. This is essential in case where column header has ellipsis (due to resize or long header text). Datagrid tooltip is displayed only on mouse hover, not on keyboard hover. If I use custom header and update tabIndex, keyboard navigation gets trapped.

A similar issue: https://github.com/mui/mui-x/issues/3892#issuecomment-1032832863 in data cell got resolved using hasFocus prop but this prop is not available for header cell.

Example: https://codesandbox.io/p/sandbox/column-header-tooltip-on-keyboard-focus-s5h7rx

Any suggestion to add this behavior?

Your environment

Search keywords: Column Header tooltip, keyboard navigation Order ID: 8813014139

michelengelen commented 4 months ago

Hey @shahidify thanks for raising an issue. The sandbox you linked is still private. Could you make it public so we can have a look? thanks! 🙇🏼

shahidify commented 4 months ago

@michelengelen Sorry, I didn't realize it's private. Updated. It should work - https://codesandbox.io/p/sandbox/column-header-tooltip-on-keyboard-focus-s5h7rx

michelengelen commented 4 months ago

This is basically a material-ui problem, since the Tooltip component apparently captures the keydown events without propagation. I am not super sure what happens here, but it works as expected when using a renderHeader without Tooltip.

@mnajdova what do you think? shouldn't this be in the material-ui repo?

mnajdova commented 4 months ago

The Tooltip components itself, by default shows the tooltip when the component receives a focus (the component in this case being the icon, no the cell). In order for this to work, the icon itself should have a tabIndex={0}, or be an IconButton component (or a custom button component).

I don't see a missing functionality on the Tooltip component iself, or please let me know if this is the case, I think this is more related to how the focus is being handled in the header cell.

Tooltip component apparently captures the keydown events without propagation

What do you mean, from what I tested it works as expected, see: https://codesandbox.io/p/sandbox/blue-sun-ng53wt?file=%2Fsrc%2FDemo.js%3A28%2C33.

michelengelen commented 4 months ago

OK, I did some more investigation into this and the root issue was that there was a tabIndex={0} set on the Typography component. Possibly to show the Tooltip when it receives focus. At the same time this did trap the keyboard events.

I did come up with a different solution though:

const CustomColumnHeader: FunctionComponent<GridColumnHeaderParams> = ({ colDef, field }) => {
  const [isOpen, setIsOpen] = React.useState(false);
  const apiRef = useGridApiContext();
  const { headerName, description } = colDef;

  React.useEffect(() => {
    setIsOpen(apiRef.current?.state.focus.columnHeader?.field === field);
  }, [apiRef.current?.state.focus.columnHeader]);

  return (
    <Tooltip
      open={isOpen}
      title={
        <Box display="flex" flexDirection="column">
          <Typography sx={{ whiteSpace: 'pre-line' }}>{description}</Typography>
        </Box>
      }
    >
      <Typography variant="body2" sx={{ fontWeight: 'bold' }}>
        {headerName}
      </Typography>
    </Tooltip>
  );
};

Now the Tootip will show when the focus state of the headers change. There is another possible solution that is more verbose:

React.useEffect(() => {
  apiRef.current.subscribeEvent('columnHeaderFocus', (params) => {
    if (field === params.field) {
      setIsOpen(true);
    }
  });
  apiRef.current.subscribeEvent('columnHeaderBlur', (params) => {
    if (field === params.field) {
      setIsOpen(false);
    }
  });
}, [apiRef]);

I do prefer the top one thoush.

@shahidify WDYT? Would that solve your issue?

shahidify commented 4 months ago

Thank you @michelengelen I will try this solution tomorrow and will let you know. Looks promising to me.

shahidify commented 4 months ago

OK, I did some more investigation into this and the root issue was that there was a tabIndex={0} set on the Typography component. Possibly to show the Tooltip when it receives focus. At the same time this did trap the keyboard events.

I did come up with a different solution though:

const CustomColumnHeader: FunctionComponent<GridColumnHeaderParams> = ({ colDef, field }) => {
  const [isOpen, setIsOpen] = React.useState(false);
  const apiRef = useGridApiContext();
  const { headerName, description } = colDef;

  React.useEffect(() => {
    setIsOpen(apiRef.current?.state.focus.columnHeader?.field === field);
  }, [apiRef.current?.state.focus.columnHeader]);

  return (
    <Tooltip
      open={isOpen}
      title={
        <Box display="flex" flexDirection="column">
          <Typography sx={{ whiteSpace: 'pre-line' }}>{description}</Typography>
        </Box>
      }
    >
      <Typography variant="body2" sx={{ fontWeight: 'bold' }}>
        {headerName}
      </Typography>
    </Tooltip>
  );
};

Now the Tootip will show when the focus state of the headers change. There is another possible solution that is more verbose:

React.useEffect(() => {
  apiRef.current.subscribeEvent('columnHeaderFocus', (params) => {
    if (field === params.field) {
      setIsOpen(true);
    }
  });
  apiRef.current.subscribeEvent('columnHeaderBlur', (params) => {
    if (field === params.field) {
      setIsOpen(false);
    }
  });
}, [apiRef]);

I do prefer the top one thoush.

@shahidify WDYT? Would that solve your issue?

@michelengelen While both these solution work for keyboard focus, I observe column headers don't display tooltip on Mouse Hover anymore. Do we need to add some additional events for mouse hover ?

michelengelen commented 4 months ago

Yes, you would need to add events for "columnHeaderOver"/"columnHeaderOut" or "columnHeaderEnter"/"columnHeaderLeave" ... whatever works best for you.

httpete commented 4 months ago

Shouldn't the Grid do this by default? Having to work around it with a custom component doesn't seem proper.

shahidify commented 3 months ago

I was out last week. I will try the suggested solution and will update. Thanks.

michelengelen commented 3 months ago

Shouldn't the Grid do this by default? Having to work around it with a custom component doesn't seem proper.

Not if you provide a custom renderer ...

shahidify commented 3 months ago

Mouse hover and keyboard focus don't seem to work together. Multiple tooltips may open (one with keyboard, one with mouse hover). Ex. - https://codesandbox.io/p/sandbox/column-header-tooltip-on-keyboard-focus-s5h7rx?file=%2Fsrc%2FCustomColumnHeader.jsx%3A25%2C8

@michelengelen any suggestion ?

shahidify commented 3 months ago

Shouldn't the Grid do this by default? Having to work around it with a custom component doesn't seem proper.

Not if you provide a custom renderer ...

The original issue is to display tooltip on keyboard focus like how it works on mouse hover. I used custom renderer to achieve the same. Will be happy if it can be achieved without custom header.

michelengelen commented 3 months ago

Shouldn't the Grid do this by default? Having to work around it with a custom component doesn't seem proper.

Not if you provide a custom renderer ...

The original issue is to display tooltip on keyboard focus like how it works on mouse hover. I used custom renderer to achieve the same. Will be happy if it can be achieved without custom header.

No, it cannot be achieved without a custom renderer atm. I will ask the team if we can achieve something like that and will update here later. 👍🏼

michelengelen commented 3 months ago

Mouse hover and keyboard focus don't seem to work together. Multiple tooltips may open (one with keyboard, one with mouse hover). Ex. - https://codesandbox.io/p/sandbox/column-header-tooltip-on-keyboard-focus-s5h7rx?file=%2Fsrc%2FCustomColumnHeader.jsx%3A25%2C8

@michelengelen any suggestion ?

You would need to only allow for one to be opened. This can be achieved by using a id state and check for this id in the open prop of the Tooltip.

const [openTooltip, setOpenTooltip] = React.useState('');
...
// on hover, keyboard focus, etc set the current targets id to the state
...
<Tooltip open={openTooltip === tooltipId} ... />

note: This is just dummy code

oliviertassinari commented 3 months ago

accessibility feature that needs to display column header tooltip on keyboard focus

I can't find in https://www.w3.org/WAI/WCAG21/Understanding/content-on-hover-or-focus.html or https://www.w3.org/TR/WCAG21/#keyboard clauses that support this requirement.

It seems that having the ellipsed text announced by screen reader is enough. I think we could even argue that it's better to not show the tooltip on focus than showing it. I would feel disoriented if a bunch of tooltips quickly appeared and disappeared as I'm using the keyboard to quickly navigate between the cells.

I suspect that the root issue is:

However, the same approach does not work on column header cell since it does not have hasFocus

https://github.com/mui/mui-x/issues/3892#issuecomment-2104969364. Maybe the the API to use the state of the data grid should be better documented, or the params of the header should be made consistent with the cells

https://github.com/mui/mui-x/blob/a4c6f4b8502923a453e2ccfce5361a0ab1b62a35/packages/x-data-grid/src/components/columnHeaders/GridColumnHeaderItem.tsx#L128

shahidify commented 3 months ago

I got a solution - https://codesandbox.io/p/sandbox/column-header-tooltip-on-keyboard-focus-s5h7rx?file=%2Fsrc%2FCustomColumnHeader.jsx%3A29%2C1

import { useEffect } from "react";

export const useColumnHeaderTooltipEvents = (apiRef, setOpenTooltipId) => {
  useEffect(() => {
    const handleFocusHover = (params) => {
      setOpenTooltipId(params.field);
    };
    const handleBlurLeave = () => {
      setOpenTooltipId(null);
    };

    const unsubscribeFocus = apiRef.current.subscribeEvent(
      "columnHeaderFocus",
      handleFocusHover
    );
    const unsubscribeBlur = apiRef.current.subscribeEvent(
      "columnHeaderBlur",
      handleBlurLeave
    );
    const unsubscribeOver = apiRef.current.subscribeEvent(
      "columnHeaderOver",
      handleFocusHover
    );
    const unsubscribeLeave = apiRef.current.subscribeEvent(
      "columnHeaderLeave",
      handleBlurLeave
    );
    const unsubscribeKeyDown = apiRef.current.subscribeEvent(
      "columnHeaderKeyDown",
      (params, e) => {
        if (e.code === "Escape") {
          setOpenTooltipId(null);
        }
      }
    );

    return () => {
      unsubscribeFocus();
      unsubscribeBlur();
      unsubscribeOver();
      unsubscribeLeave();
      unsubscribeKeyDown();
    };
  }, [apiRef, setOpenTooltipId]);
};
import { Box, Tooltip, Typography } from "@mui/material";
import { useGridApiContext } from "@mui/x-data-grid-pro";
import { useEffect, useState } from "react";
import { useColumnHeaderTooltipEvents } from "./useColumnHeaderTooltipEvents";

export const CustomColumnHeader = ({ colDef, field }) => {
  const { headerName = "", description } = colDef;
  const [openTooltipId, setOpenTooltipId] = useState(null);

  const apiRef = useGridApiContext();
  useColumnHeaderTooltipEvents(apiRef, setOpenTooltipId);

  return (
    <Tooltip
      open={openTooltipId === colDef.field}
      disableInteractive
      title={
        <Box display="flex" flexDirection="column">
          <Typography sx={{ whiteSpace: "pre-line" }}>{description}</Typography>
        </Box>
      }
    >
      <Typography variant="body2" sx={{ fontWeight: "bold" }}>
        {headerName}
      </Typography>
    </Tooltip>
  );
};

Thanks for the help @michelengelen and @oliviertassinari 👍

github-actions[bot] commented 3 months ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@shahidify: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.