Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: accessibility issue in tabs #1184

2 changes: 1 addition & 1 deletion sandpack-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"lint": "eslint '**/*.ts?(x)' --fix",
"build": "rollup -c",
"build:publish": "yarn build",
"start": "tsc -p tsconfig.esm.json --watch",
"start": "tsc -p tsconfig.json --watch",
danilowoz marked this conversation as resolved.
Show resolved Hide resolved
"dev": "storybook dev -p 6006 --quiet",
"typecheck": "tsc",
"format": "prettier --write '**/*.{ts,tsx,js,jsx}'",
Expand Down
7 changes: 6 additions & 1 deletion sandpack-react/src/components/CodeEditor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,12 @@ export const SandpackCodeEditor = forwardRef<CodeMirrorRef, CodeEditorProps>(
<SandpackStack className={classNames("editor", [className])} {...props}>
{shouldShowTabs && <FileTabs closableTabs={closableTabs} />}

<div className={classNames("code-editor", [editorClassName])}>
<div
aria-labelledby={`${activeFile}-tab`}
className={classNames("code-editor", [editorClassName])}
id={`${activeFile}-tab-panel`}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that a page can contain multiple Sandpack instances with similar content, this ID will not be unique to the page. Is there any reason why you need an ID here? There is an alternative to generate unique identifier inside Sandpack

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this ID is used inside the FileTabs component, to add the aria-controls attribute.

aria-controls={`${filePath}-tab-panel`}

is there a way I can get a unique ID and pass it between FileTabs and CodeEditor?

One way I am thinking is to add a unique id with activeFile , what's your opinion ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just introduced a new hook (useSandpackId) to your branch and I think you could use it to generate ids

role="tabpanel"
>
<CodeMirror
key={activeFile}
ref={ref}
Expand Down
4 changes: 4 additions & 0 deletions sandpack-react/src/components/CodeEditor/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ export const editorClassName = css({
padding: "$space$4 0",
},

"&:focus-within": {
outline: "-webkit-focus-ring-color auto 5px",
},

/**
* For iOS: prevent browser zoom when clicking on sandbox.
* Does NOT apply to code blocks.
Expand Down
114 changes: 100 additions & 14 deletions sandpack-react/src/components/FileTabs/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,28 @@ const tabsScrollableClassName = css({
marginBottom: "-1px",
});

const tabContainer = css({
display: "flex",
alignItems: "center",
outline: "none",
position: "relative",
paddingRight: "20px",

"&:has(button:focus)": {
outline: "-webkit-focus-ring-color auto 5px",
},
});

const closeButtonClassName = css({
padding: "0 $space$1 0 $space$1",
borderRadius: "$border$radius",
marginLeft: "$space$1",
width: "$space$5",
visibility: "hidden",
cursor: "pointer",
position: "absolute",
color: "green",
danilowoz marked this conversation as resolved.
Show resolved Hide resolved
right: "0px",

svg: {
width: "$space$3",
Expand All @@ -46,8 +62,10 @@ export const tabButton = css({
height: "$layout$headerHeight",
whiteSpace: "nowrap",

"&:focus": { outline: "none" },
[`&:hover > .${closeButtonClassName}`]: { visibility: "unset" },
"&:focus": {
outline: "none",
},
[`&:hover ~ .${closeButtonClassName}`]: { visibility: "visible" },
});

export interface FileTabsProps {
Expand All @@ -70,6 +88,7 @@ export const FileTabs = ({
const classNames = useClassNames();

const { activeFile, visibleFiles, setActiveFile } = sandpack;
const [hoveredIndex, setIsHoveredIndex] = React.useState<null | number>(null);

const handleCloseFile = (ev: React.MouseEvent<HTMLDivElement>): void => {
ev.stopPropagation();
Expand Down Expand Up @@ -111,6 +130,56 @@ export const FileTabs = ({
}
};

const onKeyDown = ({
e,
index,
}: {
e: React.KeyboardEvent<HTMLElement>;
index: number;
}) => {
const target = e.currentTarget as HTMLElement;

switch (e.key) {
case "ArrowLeft":
{
const leftSibling = target.previousElementSibling as HTMLElement;

if (leftSibling) {
leftSibling.querySelector("button")?.focus();
setActiveFile(visibleFiles[index - 1]);
}
}
break;
case "ArrowRight":
{
const rightSibling = target.nextElementSibling as HTMLElement;

if (rightSibling) {
rightSibling.querySelector("button")?.focus();
setActiveFile(visibleFiles[index + 1]);
}
}
break;
case "Home": {
const parent = target.parentElement as HTMLElement;

const firstChild = parent.firstElementChild as HTMLElement;
firstChild.querySelector("button")?.focus();
setActiveFile(visibleFiles[0]);
break;
}
case "End": {
const parent = target.parentElement as HTMLElement;
const lastChild = parent.lastElementChild as HTMLElement;
lastChild.querySelector("button")?.focus();
setActiveFile(visibleFiles[-1]);
break;
}
default:
break;
}
};

return (
<div
className={classNames("tabs", [tabsClassName, className])}
Expand All @@ -124,27 +193,44 @@ export const FileTabs = ({
])}
role="tablist"
>
{visibleFiles.map((filePath) => (
<button
key={filePath}
aria-selected={filePath === activeFile}
className={classNames("tab-button", [buttonClassName, tabButton])}
data-active={filePath === activeFile}
onClick={(): void => setActiveFile(filePath)}
role="tab"
title={filePath}
type="button"
{visibleFiles.map((filePath, index) => (
<div
className={classNames("tab-container", [tabContainer])}
onKeyDown={(e) => onKeyDown({ e, index })}
onMouseEnter={() => setIsHoveredIndex(index)}
onMouseLeave={() => setIsHoveredIndex(null)}
>
{getTriggerText(filePath)}
<button
key={filePath}
aria-controls={`${filePath}-tab-panel`}
aria-selected={filePath === activeFile}
className={classNames("tab-button", [buttonClassName, tabButton])}
data-active={filePath === activeFile}
id={`${filePath}-tab`}
onClick={(): void => setActiveFile(filePath)}
role="tab"
tabIndex={filePath === activeFile ? 0 : -1}
title={filePath}
type="button"
>
{getTriggerText(filePath)}
</button>
{closableTabs && visibleFiles.length > 1 && (
<span
className={classNames("close-button", [closeButtonClassName])}
onClick={handleCloseFile}
style={{
visibility:
filePath === activeFile || hoveredIndex === index
? "visible"
: "hidden",
}}
tabIndex={filePath === activeFile ? 0 : -1}
>
<CloseIcon />
</span>
)}
</button>
</div>
))}
</div>
</div>
Expand Down
Loading