mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
refactor: remove disabled tooltip from PaginationNavButton (#21199)
for #19974 The MUI tooltip inside `PaginationNavButton` was a controlled component. This + the stateful logic inside `PaginationNavButtonCore` meant that `showDisabledMessage` would never be set to true. I.e., the "You are already on the first page" tooltip if on the first page and the "You are already on the last page" tooltip if on the last page would never show up. The `PaginationNavButton`s gets disabled if we're at either the first/last page, and disabled buttons can't receive focus, so there's no way to open the MUI tooltips with keyboard navigation. Removing the MUI tooltip + related props from `PaginationNavButton` has no effect on my screen reader UX with macOS VoiceOver; it's entirely unchanged
This commit is contained in:
@@ -1,86 +1,28 @@
|
||||
import Tooltip from "@mui/material/Tooltip";
|
||||
import { Button } from "components/Button/Button";
|
||||
import {
|
||||
type ButtonHTMLAttributes,
|
||||
type ReactNode,
|
||||
useEffect,
|
||||
useState,
|
||||
} from "react";
|
||||
import type { ButtonHTMLAttributes, ReactNode } from "react";
|
||||
|
||||
type PaginationNavButtonProps = Omit<
|
||||
ButtonHTMLAttributes<HTMLButtonElement>,
|
||||
| "aria-disabled"
|
||||
// Need to omit color for MUI compatibility
|
||||
| "color"
|
||||
"aria-disabled"
|
||||
> & {
|
||||
// Required/narrowed versions of default props
|
||||
children: ReactNode;
|
||||
disabled: boolean;
|
||||
onClick: () => void;
|
||||
"aria-label": string;
|
||||
|
||||
// Bespoke props
|
||||
disabledMessage: ReactNode;
|
||||
disabledMessageTimeout?: number;
|
||||
};
|
||||
|
||||
function PaginationNavButtonCore({
|
||||
export function PaginationNavButton({
|
||||
onClick,
|
||||
disabled,
|
||||
disabledMessage,
|
||||
disabledMessageTimeout = 3000,
|
||||
...delegatedProps
|
||||
}: PaginationNavButtonProps) {
|
||||
const [showDisabledMessage, setShowDisabledMessage] = useState(false);
|
||||
|
||||
// Inline state sync - this is safe/recommended by the React team in this case
|
||||
if (!disabled && showDisabledMessage) {
|
||||
setShowDisabledMessage(false);
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
if (!showDisabledMessage) {
|
||||
return;
|
||||
}
|
||||
|
||||
const timeoutId = setTimeout(
|
||||
() => setShowDisabledMessage(false),
|
||||
disabledMessageTimeout,
|
||||
);
|
||||
|
||||
return () => clearTimeout(timeoutId);
|
||||
}, [showDisabledMessage, disabledMessageTimeout]);
|
||||
|
||||
return (
|
||||
<Tooltip title={disabledMessage} open={showDisabledMessage}>
|
||||
{/*
|
||||
* Going more out of the way to avoid attaching the disabled prop directly
|
||||
* to avoid unwanted side effects of using the prop:
|
||||
* - Not being focusable/keyboard-navigable
|
||||
* - Not being able to call functions in response to invalid actions
|
||||
* (mostly for giving direct UI feedback to those actions)
|
||||
*/}
|
||||
<Button
|
||||
variant="outline"
|
||||
size="icon"
|
||||
disabled={disabled}
|
||||
onClick={onClick}
|
||||
{...delegatedProps}
|
||||
/>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
|
||||
export function PaginationNavButton({
|
||||
disabledMessageTimeout = 3000,
|
||||
...delegatedProps
|
||||
}: PaginationNavButtonProps) {
|
||||
return (
|
||||
// Key prop ensures that if timeout changes, the component just unmounts and
|
||||
// remounts, avoiding a swath of possible sync issues
|
||||
<PaginationNavButtonCore
|
||||
key={disabledMessageTimeout}
|
||||
disabledMessageTimeout={disabledMessageTimeout}
|
||||
<Button
|
||||
variant="outline"
|
||||
size="icon"
|
||||
disabled={disabled}
|
||||
onClick={onClick}
|
||||
{...delegatedProps}
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -41,7 +41,6 @@ export const PaginationWidgetBase: FC<PaginationWidgetBaseProps> = ({
|
||||
return (
|
||||
<div className="flex flex-row items-center justify-center px-5 gap-x-1.5">
|
||||
<PaginationNavButton
|
||||
disabledMessage="You are already on the first page"
|
||||
disabled={isPrevDisabled}
|
||||
aria-label="Previous page"
|
||||
onClick={() => {
|
||||
@@ -68,7 +67,6 @@ export const PaginationWidgetBase: FC<PaginationWidgetBaseProps> = ({
|
||||
)}
|
||||
|
||||
<PaginationNavButton
|
||||
disabledMessage="You are already on the last page"
|
||||
disabled={isNextDisabled}
|
||||
aria-label="Next page"
|
||||
onClick={() => {
|
||||
|
||||
Reference in New Issue
Block a user