Skip to content

Commit 36311e5

Browse files
authored
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
1 parent 3d38cd5 commit 36311e5

File tree

2 files changed

+8
-68
lines changed

2 files changed

+8
-68
lines changed

site/src/components/PaginationWidget/PaginationNavButton.tsx

Lines changed: 8 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,28 @@
1-
import Tooltip from "@mui/material/Tooltip";
21
import { Button } from "components/Button/Button";
3-
import {
4-
type ButtonHTMLAttributes,
5-
type ReactNode,
6-
useEffect,
7-
useState,
8-
} from "react";
2+
import type { ButtonHTMLAttributes, ReactNode } from "react";
93

104
type PaginationNavButtonProps = Omit<
115
ButtonHTMLAttributes<HTMLButtonElement>,
12-
| "aria-disabled"
13-
// Need to omit color for MUI compatibility
14-
| "color"
6+
"aria-disabled"
157
> & {
168
// Required/narrowed versions of default props
179
children: ReactNode;
1810
disabled: boolean;
1911
onClick: () => void;
2012
"aria-label": string;
21-
22-
// Bespoke props
23-
disabledMessage: ReactNode;
24-
disabledMessageTimeout?: number;
2513
};
2614

27-
function PaginationNavButtonCore({
15+
export function PaginationNavButton({
2816
onClick,
2917
disabled,
30-
disabledMessage,
31-
disabledMessageTimeout = 3000,
32-
...delegatedProps
33-
}: PaginationNavButtonProps) {
34-
const [showDisabledMessage, setShowDisabledMessage] = useState(false);
35-
36-
// Inline state sync - this is safe/recommended by the React team in this case
37-
if (!disabled && showDisabledMessage) {
38-
setShowDisabledMessage(false);
39-
}
40-
41-
useEffect(() => {
42-
if (!showDisabledMessage) {
43-
return;
44-
}
45-
46-
const timeoutId = setTimeout(
47-
() => setShowDisabledMessage(false),
48-
disabledMessageTimeout,
49-
);
50-
51-
return () => clearTimeout(timeoutId);
52-
}, [showDisabledMessage, disabledMessageTimeout]);
53-
54-
return (
55-
<Tooltip title={disabledMessage} open={showDisabledMessage}>
56-
{/*
57-
* Going more out of the way to avoid attaching the disabled prop directly
58-
* to avoid unwanted side effects of using the prop:
59-
* - Not being focusable/keyboard-navigable
60-
* - Not being able to call functions in response to invalid actions
61-
* (mostly for giving direct UI feedback to those actions)
62-
*/}
63-
<Button
64-
variant="outline"
65-
size="icon"
66-
disabled={disabled}
67-
onClick={onClick}
68-
{...delegatedProps}
69-
/>
70-
</Tooltip>
71-
);
72-
}
73-
74-
export function PaginationNavButton({
75-
disabledMessageTimeout = 3000,
7618
...delegatedProps
7719
}: PaginationNavButtonProps) {
7820
return (
79-
// Key prop ensures that if timeout changes, the component just unmounts and
80-
// remounts, avoiding a swath of possible sync issues
81-
<PaginationNavButtonCore
82-
key={disabledMessageTimeout}
83-
disabledMessageTimeout={disabledMessageTimeout}
21+
<Button
22+
variant="outline"
23+
size="icon"
24+
disabled={disabled}
25+
onClick={onClick}
8426
{...delegatedProps}
8527
/>
8628
);

site/src/components/PaginationWidget/PaginationWidgetBase.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ export const PaginationWidgetBase: FC<PaginationWidgetBaseProps> = ({
4141
return (
4242
<div className="flex flex-row items-center justify-center px-5 gap-x-1.5">
4343
<PaginationNavButton
44-
disabledMessage="You are already on the first page"
4544
disabled={isPrevDisabled}
4645
aria-label="Previous page"
4746
onClick={() => {
@@ -68,7 +67,6 @@ export const PaginationWidgetBase: FC<PaginationWidgetBaseProps> = ({
6867
)}
6968

7069
<PaginationNavButton
71-
disabledMessage="You are already on the last page"
7270
disabled={isNextDisabled}
7371
aria-label="Next page"
7472
onClick={() => {

0 commit comments

Comments
 (0)