Skip to content

Commit

Permalink
fix(LEMS-2082): update expression popover role and remove aria-checked (
Browse files Browse the repository at this point in the history
#1371)

## Summary:
### Issue Summary
Based on a Level Access audit, the keypad button in the expression widget
- doesn't have the appropriate role
- misuses the `aria-checked` label

### Fix Summary
- Updated `Clickable` component's role to be button
- Removed `aria-checked`

Issue: LEMS-2082

## Test plan:
1. Navigate to [Expression Widget](https://prod-znd-240624-anakaren-ar-4.khanacademy.org/internal-courses/test-everything/test-everything-1/expression/e/expression-exercise)
2. Click the expression widget 
![image](https://github.com/Khan/perseus/assets/171845986/6eed2f65-796a-42e6-a837-1a1747938b39)
3. Fix reflects updated component role and does not display 'aria-checked'

### Screenshots 
#### Before 
![image](https://github.com/Khan/perseus/assets/171845986/d967771a-9d43-4386-bc52-2288719cca1c)

#### After 
![image](https://github.com/Khan/perseus/assets/171845986/3816991e-ed0b-4fec-beeb-0378a715c6c0)

Author: anakaren-rojas

Reviewers: catandthemachines, #perseus

Required Reviewers:

Approved By: catandthemachines

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), 🚫 Upload Coverage, ✅ gerald, ✅ Publish npm snapshot (ubuntu-latest, 20.x), 🚫 Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), 🚫 Jest Coverage (ubuntu-latest, 20.x), 🚫 Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1371
  • Loading branch information
anakaren-rojas committed Jun 25, 2024
1 parent 35651e0 commit ba5f334
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 7 deletions.
6 changes: 6 additions & 0 deletions .changeset/tidy-zebras-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": major
"@khanacademy/perseus-editor": major
---

update attributes for expression widget button
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ describe("expression-editor", () => {
jest.runOnlyPendingTimers();

await userEvent.click(
screen.getByRole("switch", {
screen.getByRole("button", {
name: "open math keypad",
}),
);
Expand Down
8 changes: 4 additions & 4 deletions packages/perseus/src/components/__tests__/math-input.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe("Perseus' MathInput", () => {
jest.runOnlyPendingTimers();

// Act
screen.getByRole("switch").click();
screen.getByRole("button").click();
await userEvent.click(screen.getByRole("button", {name: "1"}));
await userEvent.click(screen.getByRole("button", {name: "Plus"}));
await userEvent.click(screen.getByRole("button", {name: "2"}));
Expand Down Expand Up @@ -111,7 +111,7 @@ describe("Perseus' MathInput", () => {

// Act
// focusing the input triggers the popover
screen.getByRole("switch").click();
screen.getByRole("button").click();
await userEvent.click(screen.getByRole("button", {name: "1"}));
await userEvent.click(screen.getByRole("button", {name: "Plus"}));
await userEvent.click(screen.getByRole("button", {name: "2"}));
Expand All @@ -138,7 +138,7 @@ describe("Perseus' MathInput", () => {

// Act
// focusing the input triggers the popover
screen.getByRole("switch").click();
screen.getByRole("button").click();
await userEvent.click(screen.getByRole("button", {name: "1"}));

// Assert
Expand All @@ -160,7 +160,7 @@ describe("Perseus' MathInput", () => {

// Act
// focusing the input triggers the popover
screen.getByRole("switch").click();
screen.getByRole("button").click();
await userEvent.tab(); // to "123" tab
await userEvent.tab(); // to extra keys tab
await userEvent.tab(); // to whole keypad
Expand Down
3 changes: 1 addition & 2 deletions packages/perseus/src/components/math-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,7 @@ class InnerMathInput extends React.Component<InnerProps, State> {
? this.context.strings.closeKeypad
: this.context.strings.openKeypad
}
aria-checked={this.state.keypadOpen}
role="switch"
role="button"
onClick={() =>
this.state.keypadOpen
? this.closeKeypad()
Expand Down

0 comments on commit ba5f334

Please sign in to comment.