refactor(breadcrumbs): use menu over deprecated buttondropdown #1646
Conversation
size-limit report 📦
|
Codecov Report
@@ Coverage Diff @@
## next #1646 +/- ##
=======================================
Coverage 93.29% 93.29%
=======================================
Files 222 222
Lines 2955 2955
Branches 717 717
=======================================
Hits 2757 2757
Misses 181 181
Partials 17 17
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -133,5 +133,6 @@ export const LongTextMenu: StoryObj<Args> = { | |||
| const canvas = within(canvasElement); | |||
| const dropdownMenuTrigger = await canvas.findByRole('button'); | |||
| userEvent.click(dropdownMenuTrigger); | |||
| // TODO: capture open menu in Chromatic | |||
There was a problem hiding this comment.
For some reason this change broke the capturing of the open Menu state in Chromatic, tried the following to avail:
await new Promise((resolve) => setTimeout(resolve, 300));in the play function to wait for loadingdelay: 500in the story chromatic properties to wait before snapping
There was a problem hiding this comment.
Recommend looking at this one for comparison: https://github.com/chanzuckerberg/edu-design-system/blob/main/src/components/Menu/Menu.test.tsx#L22-L40
You might need to use an await on the click? Or we could migrate this test to a unit test to match up with the other Menu test.
There was a problem hiding this comment.
Got it snapped with the decorator padding
|
@jinlee93 on the chromatic diffs, b/c the popout menu is not attached to the directly-adjacent DOM of the breadcrumb menu, it is clipping the menu from the snapshot. We can add a decorator which includes some bottom padding to show the menu |
Looks like that was it. Will add some padding to ensure chromatic captures this... Think that resolves the need for a jest test since we're not trying to test the Menu functionality itself @booc0mtaco |
jinlee93
force-pushed
the
jlee/useMenuInBreadcrumbs
branch
from
c49ba3c to
1c565c3
Compare
EFI-1002
Summary:
Test Plan: