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

[Expression Remediation] Create arrow key navigation for TabBar component. #1384

Merged
merged 19 commits into from
Jul 3, 2024

Conversation

catandthemachines
Copy link
Member

@catandthemachines catandthemachines commented Jun 28, 2024

Summary:

Updating TabBar and TabbarItem component to support arrow key navigation.

Included in this PR

  • The creation of arrow key navigation for Tabbar and specifically the TabItems used for changing the presenting panel.
  • Implementation of functionality for tab bar items, specifically the focus property and ability to change the role type.

Issue: https://khanacademy.atlassian.net/browse/LEMS-2130

Test plan:

Storybook

@khan-actions-bot
Copy link
Contributor

khan-actions-bot commented Jun 28, 2024

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/polite-snails-know.md, packages/math-input/src/components/keypad/keypad.tsx, packages/math-input/src/components/tabbar/item.tsx, packages/math-input/src/components/tabbar/tabbar.tsx, packages/perseus/src/components/__tests__/math-input.test.tsx, packages/perseus/src/widgets/__tests__/expression-mobile.test.tsx, packages/math-input/src/components/keypad/__tests__/keypad.test.tsx, packages/math-input/src/components/keypad/__tests__/mobile-keypad.test.tsx, packages/math-input/src/components/tabbar/__tests__/item.test.tsx, packages/math-input/src/components/tabbar/__tests__/tabbar.test.tsx, packages/math-input/src/components/keypad/__tests__/__snapshots__/keypad.test.tsx.snap, packages/math-input/src/components/keypad/__tests__/__snapshots__/mobile-keypad.test.tsx.snap

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

@khan-actions-bot khan-actions-bot requested a review from a team June 28, 2024 21:23
@catandthemachines catandthemachines marked this pull request as draft June 28, 2024 21:24
Copy link
Contributor

github-actions bot commented Jun 28, 2024

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (9ddcbfb) and published it to npm. You
can install it using the tag PR1384.

Example:

yarn add @khanacademy/perseus@PR1384

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1384

Copy link
Contributor

github-actions bot commented Jun 28, 2024

Size Change: +678 B (+0.08%)

Total Size: 850 kB

Filename Size Change
packages/math-input/dist/es/index.js 80.8 kB +678 B (+0.85%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 38.2 kB
packages/kmath/dist/es/index.js 4.26 kB
packages/math-input/dist/es/strings.js 1.73 kB
packages/perseus-core/dist/es/index.js 906 B
packages/perseus-editor/dist/es/index.js 272 kB
packages/perseus-error/dist/es/index.js 866 B
packages/perseus-linter/dist/es/index.js 21.8 kB
packages/perseus/dist/es/index.js 410 kB
packages/perseus/dist/es/strings.js 3.21 kB
packages/pure-markdown/dist/es/index.js 3.67 kB
packages/simple-markdown/dist/es/index.js 12.4 kB

compressed-size-action

Copy link
Contributor

@handeyeco handeyeco left a comment

Choose a reason for hiding this comment

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

Looking good! I noticed this is still a draft, so hopefully you don't mind the early feedback. Curious about the test plan, but I think RTL can handle focus tests?

packages/math-input/src/components/tabbar/item.tsx Outdated Show resolved Hide resolved
packages/math-input/src/components/tabbar/item.tsx Outdated Show resolved Hide resolved
packages/math-input/src/components/tabbar/item.tsx Outdated Show resolved Hide resolved
packages/math-input/src/components/tabbar/item.tsx Outdated Show resolved Hide resolved
packages/math-input/src/components/tabbar/item.tsx Outdated Show resolved Hide resolved
packages/math-input/src/components/tabbar/tabbar.tsx Outdated Show resolved Hide resolved
packages/math-input/src/components/tabbar/tabbar.tsx Outdated Show resolved Hide resolved
packages/math-input/src/components/tabbar/tabbar.tsx Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 95.32710% with 5 lines in your changes missing coverage. Please review.

Project coverage is 70.98%. Comparing base (4b56e10) to head (7a50770).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1384      +/-   ##
==========================================
+ Coverage   69.97%   70.98%   +1.00%     
==========================================
  Files         494      498       +4     
  Lines      104380   104505     +125     
  Branches     5352    10648    +5296     
==========================================
+ Hits        73040    74179    +1139     
+ Misses      31224    30326     -898     
+ Partials      116        0     -116     

Impacted file tree graph

Files Coverage Δ
...ckages/math-input/src/components/keypad/keypad.tsx 96.98% <ø> (-0.02%) ⬇️
packages/math-input/src/components/tabbar/item.tsx 100.00% <100.00%> (+0.68%) ⬆️
...ckages/math-input/src/components/tabbar/tabbar.tsx 94.84% <89.36%> (-5.16%) ⬇️

... and 118 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b56e10...7a50770. Read the comment docs.

@catandthemachines catandthemachines marked this pull request as ready for review July 2, 2024 22:54
@khan-actions-bot khan-actions-bot requested a review from a team July 2, 2024 22:54
@catandthemachines
Copy link
Member Author

Looking good! I noticed this is still a draft, so hopefully you don't mind the early feedback. Curious about the test plan, but I think RTL can handle focus tests?

I added a new test file to test the focus functionality for item.tsx. I tried to add some tests in tabbar.test.tsx, but I couldn't get the mocking of the arrow-key behavior to work. Let me know if I should take another stab at it.

@khan-actions-bot khan-actions-bot requested a review from a team July 2, 2024 23:34
Copy link
Contributor

@handeyeco handeyeco left a comment

Choose a reason for hiding this comment

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

This is looking great! Really clean! I mentioned a bug I found in Slack, but after that's addressed I think we should be good to go.

@khan-actions-bot khan-actions-bot requested a review from a team July 3, 2024 18:03
Copy link
Contributor

@mark-fitzgerald mark-fitzgerald left a comment

Choose a reason for hiding this comment

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

Feels cleaner in here - thanks!

@catandthemachines catandthemachines merged commit 5de4833 into main Jul 3, 2024
9 checks passed
@catandthemachines catandthemachines deleted the catjohnson/expression-tabindex-tabbar branch July 3, 2024 20:59
SonicScrewdriver added a commit that referenced this pull request Jul 8, 2024
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @khanacademy/math-input@19.2.0

### Minor Changes

- [#1384](#1384)
[`5de483386`](5de4833)
Thanks [@catandthemachines](https://github.com/catandthemachines)! -
Updating TabBar experience in to use arrow-key navigation to access the
other TabItems. This will ensure the Expression Widget in perseus has
proper keyboard navigation for users.

## @khanacademy/perseus@24.3.0

### Minor Changes

- [#1383](#1383)
[`4b56e10de`](4b56e10)
Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - View
Locked Functions in the Interactive Graph


- [#1392](#1392)
[`b710d07db`](b710d07)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Creation of new angle graph for Mafs interactive graph widget

### Patch Changes

- [#1390](#1390)
[`7e6ccf38d`](7e6ccf3)
Thanks [@benchristel](https://github.com/benchristel)! - Internal: Move
graphing-agnostic, mathy functions in the interactive graph code to a
math/ folder.

- Updated dependencies
\[[`5de483386`](5de4833)]:
    -   @khanacademy/math-input@19.2.0

## @khanacademy/perseus-editor@7.0.3

### Patch Changes

- [#1383](#1383)
[`4b56e10de`](4b56e10)
Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - View
Locked Functions in the Interactive Graph


- [#1390](#1390)
[`7e6ccf38d`](7e6ccf3)
Thanks [@benchristel](https://github.com/benchristel)! - Internal: Move
graphing-agnostic, mathy functions in the interactive graph code to a
math/ folder.


- [#1392](#1392)
[`b710d07db`](b710d07)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Creation of new angle graph for Mafs interactive graph widget

- Updated dependencies
\[[`4b56e10de`](4b56e10),
[`7e6ccf38d`](7e6ccf3),
[`5de483386`](5de4833),
[`b710d07db`](b710d07)]:
    -   @khanacademy/perseus@24.3.0
    -   @khanacademy/math-input@19.2.0

## @khanacademy/perseus-dev-ui@1.6.1

### Patch Changes

- Updated dependencies
\[[`5de483386`](5de4833)]:
    -   @khanacademy/math-input@19.2.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants