Skip to content

Fix/keyboard accessibility#769

Open
adarshh347 wants to merge 3 commits intoaccordproject:mainfrom
adarshh347:fix/keyboard-accessibility
Open

Fix/keyboard accessibility#769
adarshh347 wants to merge 3 commits intoaccordproject:mainfrom
adarshh347:fix/keyboard-accessibility

Conversation

@adarshh347
Copy link

feat(a11y): enable keyboard accessibility for sidebar, fullscreen, and chat controls

Closes #

This PR improves keyboard accessibility across multiple interactive components in the Playground.

Several UI controls were implemented using non-semantic <div> elements or raw icons with onClick handlers only. While they functioned correctly with mouse interaction, they were not fully accessible to keyboard users or assistive technologies.

This update ensures all interactive elements can be focused via Tab and activated using Enter or Space, improving overall WCAG alignment and usability.


Changes

  • Add onKeyDown handlers to PlaygroundSidebar navigation items to support Enter and Space activation
  • Add role="button" and tabIndex={0} where necessary for semantic clarity
  • Add aria-label to the fullscreen icon in FullScreenModal
  • Add keyboard activation support to fullscreen trigger
  • Add role="button", tabIndex={0}, aria-label, and aria-pressed to AIChatPanel context toggles
  • Ensure preventDefault() is used to avoid unintended scroll behavior when pressing Space

Flags

  • No visual changes introduced
  • No business logic changes — accessibility-only enhancement
  • Behavior remains identical for mouse users
  • Manual keyboard testing performed (Tab, Enter, Space)

Screenshots or Video

N/A – No visual UI changes.
Functionality verified through keyboard navigation testing.


Related Issues


Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option
  • Vital features and changes captured in unit and/or integration tests
  • Commit messages follow AP format
  • Extend the documentation, if necessary
  • Merging to main from fork:<branchname>

@adarshh347 adarshh347 requested a review from a team as a code owner March 2, 2026 21:29
@netlify
Copy link

netlify bot commented Mar 2, 2026

Deploy Preview for ap-template-playground ready!

Name Link
🔨 Latest commit a62d846
🔍 Latest deploy log https://app.netlify.com/projects/ap-template-playground/deploys/69a602e96fc1c6000866756f
😎 Deploy Preview https://deploy-preview-769--ap-template-playground.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

…ontrols

Signed-off-by: adarshh <adarsh347k@gmail.com>
…nd chat controls

Signed-off-by: adarshh <adarsh347k@gmail.com>
… test file

Signed-off-by: adarshh <adarsh347k@gmail.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses Issue #768 by improving keyboard accessibility across several UI components in the Template Playground. It adds onKeyDown handlers, role="button", tabIndex={0}, aria-label, and aria-pressed attributes to interactive elements that were previously only mouse-operable.

Changes:

  • PlaygroundSidebar.tsx: Adds onKeyDown handlers (Enter/Space) to top and bottom nav items, with preventDefault() to avoid scroll on Space.
  • FullScreenModal.tsx: Adds role="button", tabIndex={0}, aria-label, and onKeyDown to the fullscreen trigger icon.
  • AIChatPanel.tsx: Adds role="button", tabIndex={0}, aria-label, aria-pressed, and onKeyDown to the three context toggle divs.
  • KeyboardAccessibility.test.tsx (new): Unit tests for keyboard activation of sidebar nav items.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/components/PlaygroundSidebar.tsx Adds onKeyDown handlers to top and bottom nav item wrappers
src/components/FullScreenModal.tsx Adds keyboard accessibility attributes and handler to the MdFullscreen icon
src/components/AIChatPanel.tsx Adds keyboard accessibility attributes and handlers to the three context toggle <div> elements
src/tests/components/KeyboardAccessibility.test.tsx New test file covering keyboard activation of sidebar items
Comments suppressed due to low confidence (1)

src/components/PlaygroundSidebar.tsx:184

  • The "Fullscreen" nav item wrapper (div[role="button"]) has no onClick handler because the Fullscreen entry in navTop (line 127) omits the onClick property. As a result, when a keyboard user focuses the outer wrapper and presses Enter or Space, onClick?.() is a no-op — nothing happens.

Meanwhile, FullScreenModal renders its own MdFullscreen icon that is now also role="button" with tabIndex={0}. This creates two nested interactive elements for the same action, which is invalid per WCAG 4.1.3 (nested interactive elements). A screen reader and keyboard user will encounter an outer "Fullscreen" button that does nothing and an inner "Open fullscreen preview" button that works.

To fix this, the Fullscreen nav item in navTop should either:

  • Provide an onClick that delegates to the fullscreen modal's open action (lifting state up), or
  • Remove tabIndex={0} and role="button" from the FullScreenModal's MdFullscreen icon (since the outer wrapper now handles keyboard), and instead pass the open handler through props.

Currently, the outer nav item wrapper for "Fullscreen" provides no keyboard functionality despite being marked as a button.

          <div
            role="button"
            aria-label={title}
            tabIndex={0}
            onClick={onClick}
            onKeyDown={(e) => {
              if (e.key === 'Enter' || e.key === ' ') {
                e.preventDefault();
                onClick?.();
              }
            }}
            className={`group playground-sidebar-nav-item ${
              active ? 'playground-sidebar-nav-item-active' : 'playground-sidebar-nav-item-inactive'
            } tour-${title.toLowerCase().replace(' ', '-')}`}
          >
            {component ? (
              <div className="playground-sidebar-nav-item-icon-container">
                {component}
              </div>
            ) : Icon ? (
              <Icon size={20} />
            ) : null}
            <span className="playground-sidebar-nav-item-title">{title}</span>
          </div>

Comment on lines +41 to +51
role="button"
tabIndex={0}
aria-label="Open fullscreen preview"
style={{ cursor: "pointer" }}
onClick={() => setOpen(true)}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setOpen(true);
}
}}
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

The FullScreenModal's MdFullscreen icon now has role="button", tabIndex={0}, and aria-label attributes, but it is rendered inside the "Fullscreen" nav item container in PlaygroundSidebar, which also has role="button" and tabIndex={0}. This creates a nested interactive elements structure — a role="button" element inside another role="button" — which is invalid per the ARIA spec (interactive controls cannot be nested inside each other). Screen readers and assistive technologies may handle this unpredictably.

Suggested change
role="button"
tabIndex={0}
aria-label="Open fullscreen preview"
style={{ cursor: "pointer" }}
onClick={() => setOpen(true)}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
setOpen(true);
}
}}
style={{ cursor: "pointer" }}
onClick={() => setOpen(true)}

Copilot uses AI. Check for mistakes.
Comment on lines +42 to +124
describe("Keyboard Accessibility", () => {
beforeEach(() => {
vi.clearAllMocks();
});

describe("PlaygroundSidebar - Top Nav", () => {
it("activates Editor button on Enter key", () => {
render(<PlaygroundSidebar />);
const editorButton = screen.getByRole("button", { name: /Editor/i });
fireEvent.keyDown(editorButton, { key: "Enter" });
expect(mockSetEditorsVisible).toHaveBeenCalled();
});

it("activates Editor button on Space key", () => {
render(<PlaygroundSidebar />);
const editorButton = screen.getByRole("button", { name: /Editor/i });
fireEvent.keyDown(editorButton, { key: " " });
expect(mockSetEditorsVisible).toHaveBeenCalled();
});

it("does not activate Editor button on other keys", () => {
render(<PlaygroundSidebar />);
const editorButton = screen.getByRole("button", { name: /Editor/i });
fireEvent.keyDown(editorButton, { key: "Tab" });
expect(mockSetEditorsVisible).not.toHaveBeenCalled();
});

it("activates Preview button on Enter key", () => {
render(<PlaygroundSidebar />);
const previewButton = screen.getByRole("button", { name: /Preview/i });
fireEvent.keyDown(previewButton, { key: "Enter" });
expect(mockSetPreviewVisible).toHaveBeenCalled();
});

it("activates Problems button on Space key", () => {
render(<PlaygroundSidebar />);
const problemsButton = screen.getByRole("button", { name: /Problems/i });
fireEvent.keyDown(problemsButton, { key: " " });
expect(mockSetProblemPanelVisible).toHaveBeenCalled();
});

it("activates AI Assistant button on Enter key", () => {
render(<PlaygroundSidebar />);
const aiButton = screen.getByRole("button", { name: /AI Assistant/i });
fireEvent.keyDown(aiButton, { key: "Enter" });
expect(mockSetAIChatOpen).toHaveBeenCalled();
});
});

describe("PlaygroundSidebar - Bottom Nav", () => {
it("activates Share button on Enter key", () => {
render(<PlaygroundSidebar />);
const shareButton = screen.getByRole("button", { name: /Share/i });
fireEvent.keyDown(shareButton, { key: "Enter" });
// Share calls handleShare which is async, just verify no error
expect(shareButton).toBeInTheDocument();
});

it("activates Settings button on Space key", () => {
render(<PlaygroundSidebar />);
const settingsButton = screen.getByRole("button", { name: /Settings/i });
fireEvent.keyDown(settingsButton, { key: " " });
expect(mockSetSettingsOpen).toHaveBeenCalledWith(true);
});

it("activates Start Tour button on Enter key", () => {
render(<PlaygroundSidebar />);
const tourButton = screen.getByRole("button", { name: /Start Tour/i });
fireEvent.keyDown(tourButton, { key: "Enter" });
expect(tourButton).toBeInTheDocument();
});
});

describe("PlaygroundSidebar - focusability", () => {
it("all nav items have tabIndex=0 for keyboard focus", () => {
render(<PlaygroundSidebar />);
const buttons = screen.getAllByRole("button");
buttons.forEach((button) => {
expect(button).toHaveAttribute("tabindex", "0");
});
});
});
});
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

The keyboard accessibility changes for AIChatPanel (context toggle buttons) and FullScreenModal (fullscreen trigger) are not covered by any unit tests. The new test file only tests PlaygroundSidebar keyboard accessibility. Given that the PR description states "Vital features and changes captured in unit and/or integration tests" is unchecked in the author checklist, and that the codebase has unit tests for components like SettingsModal and PlaygroundSidebar, tests for the onKeyDown handlers in AIChatPanel and FullScreenModal should be added.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +92 to +112
it("activates Share button on Enter key", () => {
render(<PlaygroundSidebar />);
const shareButton = screen.getByRole("button", { name: /Share/i });
fireEvent.keyDown(shareButton, { key: "Enter" });
// Share calls handleShare which is async, just verify no error
expect(shareButton).toBeInTheDocument();
});

it("activates Settings button on Space key", () => {
render(<PlaygroundSidebar />);
const settingsButton = screen.getByRole("button", { name: /Settings/i });
fireEvent.keyDown(settingsButton, { key: " " });
expect(mockSetSettingsOpen).toHaveBeenCalledWith(true);
});

it("activates Start Tour button on Enter key", () => {
render(<PlaygroundSidebar />);
const tourButton = screen.getByRole("button", { name: /Start Tour/i });
fireEvent.keyDown(tourButton, { key: "Enter" });
expect(tourButton).toBeInTheDocument();
});
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

The tests for "Share" (lines 92-98) and "Start Tour" (lines 107-112) fire keyDown events but their assertions only verify that the button element is in the document (expect(button).toBeInTheDocument()). This does not actually verify that the keyboard handler was triggered or that any action occurred. Such tests will always pass regardless of whether the onKeyDown handler works or not.

For the "Share" test, navigator.clipboard.writeText could be mocked to verify it was called. For the "Start Tour" test, the Tour mock is already set up and Tour.start could be asserted to have been called (vi.mock("../../components/Tour", ...)). At minimum, the comment "Share calls handleShare which is async, just verify no error" acknowledges this but the same weak pattern is applied to the Tour test without explanation.

Copilot generated this review using guidance from repository custom instructions.
Copy link
Member

@sanketshevkar sanketshevkar left a comment

Choose a reason for hiding this comment

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

@adarshh347 thanks for raising this PR. Is it possible for you to please include a a11y report before and after your changes? You can use axe devtool to generate the report

https://chromewebstore.google.com/detail/axe-devtools-web-accessib/lhdoppojpmngadmnindnejefpokejbdd?pli=1

I'd suggest if there are multiple issues identified and changes spanning across a large number of files. Its better to divide the tasks down to small issues and raise PRs separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants