-
Notifications
You must be signed in to change notification settings - Fork 78
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
fix(tab-nav): emit calciteTabChange
event only when selected tab changes
#11812
base: dev
Are you sure you want to change the base?
Conversation
calciteTabChange
event only when selected tab changes
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.
Looking good, @anveshmekala! 😎
@@ -37,7 +37,6 @@ describe("calcite-tab-nav", () => { | |||
const activeEventSpy = await page.spyOnEvent("calciteTabChange"); | |||
const firstTabTitle = await page.find("calcite-tab-title"); | |||
|
|||
firstTabTitle.setProperty("selected", true); |
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.
We still need this to ensure it doesn’t emit when selecting tab titles programmatically, right?
@@ -1,6 +1,7 @@ | |||
export interface TabChangeEventDetail { | |||
/** The tab ID that just became selected */ | |||
tab: number | string; | |||
isUserTriggered?: 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.
Nitpick: userTriggered
?
@@ -47,7 +46,7 @@ describe("calcite-tab-nav", () => { | |||
|
|||
await page.keyboard.press("Enter"); | |||
await page.waitForChanges(); | |||
expect(activeEventSpy).toHaveReceivedEventTimes(2); | |||
expect(activeEventSpy).toHaveReceivedEventTimes(1); |
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.
Can you update the test to cover emitting for both keyboard and mouse interactions? The current version won’t fail if one of the two interactions stops emitting.
Related Issue: #9792
Summary
No longer emits
calciteTabChange
event when selected tab is activated again via mouse or keyboard.