Skip to content

Commit

Permalink
feat(react): remove the default outline for tabindex="-1" for acces…
Browse files Browse the repository at this point in the history
…sibility reasons (#694)

feat: remove the default outline for tabindex="-1" for accessibility reasons
  • Loading branch information
cheton authored Feb 18, 2023
1 parent a28f2da commit e4a5f73
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 31 deletions.
5 changes: 3 additions & 2 deletions packages/react/src/drawer/DrawerContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ const DrawerContent = forwardRef((
contentRef, // internal use only
} = { ...drawerContext };
const combinedRef = useMergeRefs(contentRef, ref);
const styleProps = useDrawerContentStyle({ placement, size });
const tabIndex = -1;
const styleProps = useDrawerContentStyle({ placement, size, tabIndex });
const contentProps = {
ref: combinedRef,
role: 'dialog',
tabIndex: -1,
tabIndex,
onClick: event => event.stopPropagation(),
onKeyDown: event => {
if (event.key === 'Escape') {
Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/drawer/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,15 @@ const useDrawerOverlayStyle = () => {
const useDrawerContentStyle = ({
placement = defaultPlacement,
size = defaultSize,
tabIndex,
}) => {
const isLeftOrRight = (placement === 'left' || placement === 'right');
const [colorMode] = useColorMode();
const [colorStyle] = useColorStyle({ colorMode });
const baseStyle = {
display: 'flex',
flexDirection: 'column',
outline: (tabIndex < 0) ? 0 : undefined, // Remove the default outline for tabindex="-1"
overflow: 'clip', // Set overflow to clip to forbid all scrolling for drawer content
position: 'relative',
};
Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/menu/MenuContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ const MenuContent = forwardRef((
ensureFunction(onKeyDown)(event);
};

const styleProps = useMenuContentStyle();
const tabIndex = -1;
const styleProps = useMenuContentStyle({ tabIndex });

const eventHandlers = {
onBlur: callEventHandlers(onBlurProp, handleBlur),
Expand Down Expand Up @@ -128,7 +129,7 @@ const MenuContent = forwardRef((
placement={placement}
ref={menuRef}
role="menu"
tabIndex={-1}
tabIndex={tabIndex}
unmountOnExit={true}
usePortal={false} // Pass `true` in `PopperProps` to render menu in a portal
willUseTransition={true}
Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/menu/MenuItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ const MenuItem = forwardRef((
closeOnSelect,
onClose: closeMenu,
} = { ...menuContext };
const styleProps = useMenuItemStyle();
const tabIndex = -1;
const styleProps = useMenuItemStyle({ tabIndex });

return (
<Box
ref={ref}
role={role}
tabIndex={-1}
tabIndex={tabIndex}
disabled={disabled}
aria-disabled={ariaAttr(disabled)}
onClick={callEventHandlers(onClick, event => {
Expand Down
18 changes: 8 additions & 10 deletions packages/react/src/menu/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@ const useMenuButtonStyle = () => {
};
};

const useMenuContentStyle = () => {
const useMenuContentStyle = ({
tabIndex,
}) => {
return {
// No style for menu content
outline: (tabIndex < 0) ? 0 : undefined, // Remove the default outline for tabindex="-1"
};
};

Expand All @@ -76,9 +78,6 @@ const useMenuListStyle = () => {
m: '0',
p: '0',
py: '2x',
_focus: {
outline: 'none',
},
};
};

Expand All @@ -96,7 +95,9 @@ const useMenuGroupStyle = () => {
};
};

const useMenuItemStyle = () => {
const useMenuItemStyle = ({
tabIndex,
}) => {
const theme = useTheme();
const [colorMode] = useColorMode();
const color = {
Expand Down Expand Up @@ -127,7 +128,7 @@ const useMenuItemStyle = () => {
textDecoration: 'none',
alignItems: 'center',
textAlign: 'left',
outline: 'none',
outline: (tabIndex < 0) ? 0 : undefined, // Remove the default outline for tabindex="-1"
px: '3x',
py: '2x',
userSelect: 'none',
Expand Down Expand Up @@ -229,9 +230,6 @@ const useSubmenuListStyle = ({
m: '0',
p: '0',
py: '2x',
_focus: {
outline: 'none',
},
};
};

Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/modal/ModalContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ const ModalContent = forwardRef((
placement, // internal use only
} = { ...modalContext };
const combinedRef = useMergeRefs(contentRef, ref);
const styleProps = useModalContentStyle({ placement, scrollBehavior, size });
const tabIndex = -1;
const styleProps = useModalContentStyle({ placement, scrollBehavior, size, tabIndex });
const contentProps = {
ref: combinedRef,
role: 'dialog',
tabIndex: -1,
tabIndex,
onClick: event => event.stopPropagation(),
onKeyDown: event => {
if (event.key === 'Escape') {
Expand Down
4 changes: 3 additions & 1 deletion packages/react/src/modal/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ const useModalContentStyle = ({
placement, // No default value if not specified
scrollBehavior, // No default value if not specified
size = defaultSize,
tabIndex,
}) => {
const [colorMode] = useColorMode();
const [colorStyle] = useColorStyle({ colorMode });
const baseStyle = {
display: 'flex',
flexDirection: 'column',
overflow: 'clip', // Set overflow to clip to forbid all scrolling for modal content
outline: (tabIndex < 0) ? 0 : undefined, // Remove the default outline for tabindex="-1"
overflow: 'clip', // Set overflow to clip to prevent any scrolling in the modal content
position: 'relative',
};
const colorModeStyle = {
Expand Down
4 changes: 3 additions & 1 deletion packages/react/src/popover/PopoverContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ const PopoverContent = ({
mousePageY,
arrowAt,
} = usePopover();
const styleProps = usePopoverContentStyle();
const tabIndex = -1;
const styleProps = usePopoverContentStyle({ tabIndex });
const mouseLeaveTimeoutRef = useRef();
let eventHandlers = {};
let roleProps = {};
Expand Down Expand Up @@ -214,6 +215,7 @@ const PopoverContent = ({
return (
<Box
ref={ref}
tabIndex={tabIndex}
{...styleProps}
{...transitionStyle}
transformOrigin={mapPlacementToTransformOrigin(placement)}
Expand Down
9 changes: 4 additions & 5 deletions packages/react/src/popover/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ const usePopoverTriggerStyle = () => {
};
};

const usePopoverContentStyle = () => {
const usePopoverContentStyle = ({
tabIndex,
}) => {
const [colorMode] = useColorMode();
const [colorStyle] = useColorStyle({ colorMode });
const backgroundColor = {
Expand All @@ -23,7 +25,6 @@ const usePopoverContentStyle = () => {
backgroundColor,
color,
boxShadow: colorStyle?.shadow?.thin,
tabIndex: '-1',
borderWidth: 1,
fontSize: 'sm',
lineHeight: 'sm',
Expand All @@ -33,9 +34,7 @@ const usePopoverContentStyle = () => {
flexDirection: 'column',
borderRadius: 'sm',
maxWidth: '288px',
_focus: {
outline: 0,
},
outline: (tabIndex < 0) ? 0 : undefined, // Remove the default outline for tabindex="-1"
};
};

Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/tabs/Tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ const Tab = forwardRef((
const isSelected = isIndexEqual(index, context?.index);
const orientation = context?.orientation;
const variant = context?.variant;
const styleProps = useTabStyle({ disabled, isSelected, orientation, variant });
const tabIndex = (disabled || isSelected) ? -1 : 0;
const styleProps = useTabStyle({ disabled, isSelected, orientation, variant, tabIndex });
const handleClick = callEventHandlers(onClick, (event) => {
if (isSelected) {
// Do not trigger onChange if the tab is already selected
Expand Down Expand Up @@ -63,7 +64,7 @@ const Tab = forwardRef((
onClick: handleClick,
ref,
role: 'tab',
tabIndex: (disabled || isSelected) ? -1 : 0,
tabIndex,
...styleProps,
...rest,
});
Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/tabs/TabPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ const TabPanel = forwardRef((
const tabId = `${config.name}:Tab-${index}`;
const tabPanelId = `${config.name}:TabPanel-${index}`;
const isSelected = isIndexEqual(index, context?.index);
const styleProps = useTabPanelStyle({ isSelected });
const tabIndex = 0;
const styleProps = useTabPanelStyle({ tabIndex });

// Ensure the tab panel is registered only once at the first render
useEffectOnce(() => {
Expand All @@ -49,7 +50,7 @@ const TabPanel = forwardRef((
id: tabPanelId,
ref,
role: 'tabpanel',
tabIndex: 0,
tabIndex,
...styleProps,
...rest,
});
Expand Down
9 changes: 7 additions & 2 deletions packages/react/src/tabs/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const useTabStyle = ({
disabled,
isSelected,
orientation,
tabIndex,
variant,
}) => {
const theme = useTheme();
Expand Down Expand Up @@ -120,6 +121,7 @@ const useTabStyle = ({
alignItems: 'center',
px: '3x',
py: '2x',
outline: (tabIndex < 0) ? 0 : undefined, // Remove the default outline for tabindex="-1"
_hover: {
border: 'none',
[selectedBorderColorKey]: getBorderColorStyleWithFallback(hoverBorderColor),
Expand Down Expand Up @@ -336,9 +338,12 @@ const useTabListStyle = ({
};

const useTabPanelStyle = ({
isSelected,
tabIndex,
}) => {
return {};
return {
// Remove the default outline for accessibility reasons, even when the TabPanel is focused using the keyboard (regardless of the tabIndex value specified on the TabPanel).
outline: 0,
};
};

export {
Expand Down

0 comments on commit e4a5f73

Please sign in to comment.