-
Notifications
You must be signed in to change notification settings - Fork 10
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
WB-1588: Allow custom ActionItem elements #2135
Conversation
🦋 Changeset detectedLatest commit: 4533719 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
Size Change: -40 B (0%) Total Size: 90.9 kB
ℹ️ View Unchanged
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (002aeb6) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2135" Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2135 |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2135 +/- ##
==========================================
- Coverage 97.05% 97.05% -0.01%
==========================================
Files 241 241
Lines 28038 28006 -32
Branches 2464 2405 -59
==========================================
- Hits 27212 27180 -32
Misses 826 826
Continue to review full report in Codecov by Sentry.
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-rypjmjkakz.chromatic.com/ Chromatic results:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really sharp - thank you for working on this!
wrapper: { | ||
// This removes the 300ms click delay on mobile browsers by indicating | ||
// that "double-tap to zoom" shouldn't be used on this element. | ||
touchAction: "manipulation", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh - neat!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: Added separate docs for ActionItem
to help understanding better how this component works.
@@ -431,3 +442,91 @@ const locales = [ | |||
{id: "fr", locale: "fr", localName: "français"}, | |||
{id: "it", locale: "it", localName: "italiano"}, | |||
]; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: This story shows how we can use an ActionMenu
with custom ActionItem
elements.
@@ -10,7 +10,7 @@ import {render, screen} from "@testing-library/react"; | |||
import userEvent from "@testing-library/user-event"; | |||
|
|||
import plus from "@phosphor-icons/core/regular/plus.svg"; | |||
import {ActionItem, OptionItem} from "@khanacademy/wonder-blocks-dropdown"; | |||
import {OptionItem} from "@khanacademy/wonder-blocks-dropdown"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: Removed ActionItem
from here as we now delegate the ActionItem's clickable behavior/rendering to CompactCell
.
packages/wonder-blocks-cell/src/components/internal/cell-core.tsx
Outdated
Show resolved
Hide resolved
@@ -18,7 +18,10 @@ describe("ActionItem", () => { | |||
render(<ActionItem href="/foo" label="Example" disabled={true} />); | |||
|
|||
// Assert | |||
expect(screen.getByRole("menuitem")).toBeDisabled(); | |||
expect(screen.getByRole("menuitem")).toHaveAttribute( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: Cell handles the disabled state with the aria-disabled
attr.
* https://khanacademy.org/math/algebra/eval-exprs will trigger a full | ||
* page reload. | ||
*/ | ||
skipClientNav?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: This is no longer used, so I got rid of it.
Summary:
ActionItem
components by usingCell
internally.ClickableBehavior
fromActionItem
and replaced it withCompactCell
(which internally usesClickable
).skipClientNav
fromActionItem
as it is no longer used/needed.ActionMenu
andActionItem
.ActionItem
changes:role
is used to set therole
attribute on the cell's root element.rootStyle
is used to override thestyle
attribute on the cell's rootelement.
Issue: WB-1588
Test plan:
ActionMenu:
is called as expected.
Screen.Recording.2023-12-05.at.12.51.05.PM.mov
ActionItem docs:
ActionItem
is correct and up to date.