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

1 change: 0 additions & 1 deletion sandpack-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
"lint": "eslint '**/*.ts?(x)' --fix",
"build": "rollup -c",
"build:publish": "yarn build",
"start": "tsc -p tsconfig.esm.json --watch",
"dev": "storybook dev -p 6006 --quiet",
"typecheck": "tsc",
"format": "prettier --write '**/*.{ts,tsx,js,jsx}'",
Expand Down
26 changes: 15 additions & 11 deletions sandpack-react/src/Playground.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,23 @@ export const Basic: React.FC = () => {
return (
<div style={{ height: "400vh" }}>
<Sandpack
customSetup={{
dependencies: {
"react-content-loader": "latest",
"radix-ui": "latest",
"styled-components": "latest",
"react-dom": "latest",
react: "latest",
"react-table": "latest",
},
}}
options={{
bundlerURL: "https://ymxnqs-3000.csb.app",
showTabs: true,
closableTabs: true,
}}
// customSetup={{
// dependencies: {
// "react-content-loader": "latest",
// "radix-ui": "latest",
// "styled-components": "latest",
// "react-dom": "latest",
// react: "latest",
// "react-table": "latest",
// },
// }}
// options={{
// bundlerURL: "https://ymxnqs-3000.csb.app",
// }}
template="react"
/>
</div>
Expand Down
17 changes: 15 additions & 2 deletions sandpack-react/src/components/CodeEditor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useActiveCode } from "../../hooks/useActiveCode";
import { useSandpack } from "../../hooks/useSandpack";
import type { CustomLanguage, SandpackInitMode } from "../../types";
import { useClassNames } from "../../utils/classNames";
import { useSandpackId } from "../../utils/useAsyncSandpackId";
import { FileTabs } from "../FileTabs";
import { RunButton } from "../common/RunButton";
import { SandpackStack } from "../common/Stack";
Expand Down Expand Up @@ -94,11 +95,23 @@ export const SandpackCodeEditor = forwardRef<CodeMirrorRef, CodeEditorProps>(
updateCode(newCode, shouldUpdatePreview);
};

const activeFileUniqueId = useSandpackId();

return (
<SandpackStack className={classNames("editor", [className])} {...props}>
{shouldShowTabs && <FileTabs closableTabs={closableTabs} />}
{shouldShowTabs && (
<FileTabs
activeFileUniqueId={activeFileUniqueId}
closableTabs={closableTabs}
/>
)}

<div className={classNames("code-editor", [editorClassName])}>
<div
aria-labelledby={`${activeFile}-${activeFileUniqueId}-tab`}
className={classNames("code-editor", [editorClassName])}
id={`${activeFile}-${activeFileUniqueId}-tab-panel`}
role="tabpanel"
>
<CodeMirror
key={activeFile}
ref={ref}
Expand Down
3 changes: 3 additions & 0 deletions sandpack-react/src/components/CodeEditor/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export const editorClassName = css({
[`.${placeholderClassName}`]: {
padding: "$space$4 0",
},
"&:focus-within": {
outline: "$colors$accent auto 1px",
Copy link
Member

Choose a reason for hiding this comment

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

@shrilakshmishastry, this can be implemented on your end, meaning inside the application that implements Sandpack, instead of adding a default style that might not be wanted by everyone. For a basic and generic experience, the editor already provides the necessary hint when it's focused, so I don't think adding such an opinionated style will be a good idea now.

For example, this can be added to your application:

.sp-code-editor:focus-within  {
  outline: 1px auto red;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, got it 👍 make sense, I will revert this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

},

/**
* For iOS: prevent browser zoom when clicking on sandbox.
Expand Down
6 changes: 5 additions & 1 deletion sandpack-react/src/components/CodeEditor/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ export const getEditorTheme = (): Extension =>
outline: "none",
},

".cm-activeLine": {
"& .cm-activeLine": {
backgroundColor: "transparent",
},

"&.cm-editor.cm-focused .cm-activeLine": {
backgroundColor: `var(--${THEME_PREFIX}-colors-surface3)`,
borderRadius: `var(--${THEME_PREFIX}-border-radius)`,
},
Expand Down
119 changes: 105 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",
margin: "1px 0",

"&:has(button:focus)": {
outline: "$colors$accent auto 1px",
},
});

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",
right: "0px",

svg: {
width: "$space$3",
Expand All @@ -46,15 +62,22 @@ 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 {
/**
* This adds a close button next to each file with a unique trigger to close it.
*/
closableTabs?: boolean;
/**
* unique id appended with active files. This is
* used in aria-controls value along the combination of activeFile
*/
activeFileUniqueId: string;
}

/**
Expand All @@ -70,6 +93,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 +135,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 +198,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}-${props.activeFileUniqueId}-tab-panel`}
aria-selected={filePath === activeFile}
className={classNames("tab-button", [buttonClassName, tabButton])}
data-active={filePath === activeFile}
id={`${filePath}-${props.activeFileUniqueId}-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
9 changes: 9 additions & 0 deletions sandpack-react/src/utils/useAsyncSandpackId.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@ import { useId as useReactId } from "react";

import { generateRandomId } from "./stringUtils";

export const useSandpackId = () => {
if (typeof useReactId === "function") {
/* eslint-disable-next-line */
return useReactId();
} else {
return generateRandomId();
}
};

/**
* This is a hard constraint to make URLs shorter.
* For example, this id will be used to mount SW in the iframe
Expand Down
Loading