fix: accessibility issue in tabs

[shrilakshmishastry]

Issue:

• FileTabs has given the role tab, but the tab accessibility pattern is not taken care.

Issues noticed:

• Not highlighting the tab when the file name is focused
• Not highlighting the focus state of the editor
• cross icon or close button not keyboard accessible

Fixed:

• basic tab accessibility pattern :

  1. on the tab click to focus on the first tab item initially and on the previously focused tab in case it's accessed earlier
  2. Managing Focus Within Components Using a Roving tabindex
  3. highlighting the focus on the tab and editor by default outline of the browser when focused

• on the focus of the tab, close CTA is visible and is keyboard accessible

Screen recording

Tab pattern implementation
https://github.com/user-attachments/assets/6221d6dd-1050-486c-adb6-9e2e6b1420f2

Editor accessibility
https://github.com/user-attachments/assets/7049baa6-762d-4015-85b6-21fd20a20f41

What kind of change does this pull request introduce?

• Accessibility fix

Checklist

• Documentation; N/A
• Storybook (if applicable);
• Tests;
• Ready to be merged;

This fix partially #1173

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[vercel]

@shrilakshmishastry is attempting to deploy a commit to the CodeSandbox Team on Vercel.

A member of the Team first needs to authorize it.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sandpack-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 4, 2024 7:54am

[danilowoz]

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

[shrilakshmishastry]

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

sandpack/sandpack-react/src/components/FileTabs/index.tsx

Line 205 in e571870

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 ?

[danilowoz]

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

[shrilakshmishastry]

Hey @danilowoz 👋 Thanks for reviewing !! Did you get a chance to try it out ?
I am not satisfied with how I handled the outline when the tab and code editor are focused , using the default browser outline. Please suggest how to improve this.

[danilowoz]

I'm a bit hesitant about adding a ring focus over the code editor, giving the following reasons:

• There is already a visual focus indication in the component (active line), and I just improved to remove the style when there is no focus;
• It might disturb the layout once the editor wasn't designed to contain borders except for the border that wrap everything
• If your project really needs a more prominent focus indication, you can always implement it on your side by overwriting the default styles.

Let me know what you think :)

[shrilakshmishastry]

I'm a bit hesitant about adding a ring focus over the code editor, giving the following reasons:

• There is already a visual focus indication in the component (active line), and I just improved to remove the style when there is no focus;
• It might disturb the layout once the editor wasn't designed to contain borders except for the border that wrap everything
• If your project really needs a more prominent focus indication, you can always implement it on your side by overwriting the default styles.

Let me know what you think :)

Hey 👋 ,

1. I tried focusing the editor using a tab but could not see the focus highlight on both dark and light modes.
2. focusing is outline right, it's not going to affect any layout , correct me here please
3. can you please explain to me the last point a bit more?

[shrilakshmishastry]

feat: add unique id to each tab and tab panel for aria-* attributes

1. I used useSandpackId() in this commit to generate unique values for the aria-labelledby and aria-controls attributes.

2. added back outline to the editor on focus

   sandpack/sandpack-react/src/components/CodeEditor/styles.ts

   Line 31 in a127706

   outline: "$colors$accent auto 1px",

[danilowoz]

@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;
}

[shrilakshmishastry]

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

[shrilakshmishastry]

reverted chore: remove focus-within added in codeEditor/styles

[shrilakshmishastry]

[danilowoz]

It looks good to me! But could you please check the Typescript job and fix the issues?

[shrilakshmishastry]

In the refactor: make activeFileUniqueId as optional paramater in FileTabs commit,

1. I have made activeFileUniqueId as optional. So it doesn't break the existing consumption
2. fixed the ts type issue, it was due to not passing this prop in codeviewer component
3. moved aria-* to div level in FIleTab, this is done as voice-over was not considering tab as in a series of tabs when it was added to button.

@danilowoz can you please re-review ?