-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(react-utilities): enhance useFirstMount hook to support React concurrent mode with useEffect for first mount tracking #34985
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
base: master
Are you sure you want to change the base?
Changes from all commits
8fb7f0e
56dd12b
802239d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { realPress } from 'cypress-real-events/commands/realPress'; | ||
|
||
/** | ||
* Press command fallback for Cypress 13 compatibility. | ||
* The press command is available in Cypress 14+ but not in Cypress 13. | ||
*/ | ||
Cypress.Commands.add('press', realPress); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
import 'cypress-real-events'; | ||
import './commands'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "minor", | ||
"comment": "feat: enhance hook to support React concurrent mode using setEffect for first mount tracking", | ||
"packageName": "@fluentui/react-utilities", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,12 +23,12 @@ describe('Keyborg', () => { | |
|
||
dmytrokirpa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
it('should dispose keyborg instance on unmount', () => { | ||
mount(<Example />); | ||
cy.window().then(win => { | ||
cy.window().should(win => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
just to clarify, these were failing all along mainly because of this ? or without the fix to useFirstMount it would still fail ? |
||
// @ts-expect-error - Only way to definitively check if keyborg is disposed | ||
expect(win.__keyborg).not.equals(undefined); | ||
}); | ||
mount(<div>Unmounted</div>); | ||
cy.window().then(win => { | ||
cy.window().should(win => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we bring some lint rule to disable using |
||
// @ts-expect-error - Only way to definitively check if keyborg is disposed | ||
expect(win.__keyborg).equals(undefined); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import 'cypress-real-events'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't this loaded via our baseconfig ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure, it's imported in the Tree component in the same package, but not for the FlatTree component. I'll double check if it's needed |
||
import * as React from 'react'; | ||
import { mount as mountBase } from '@cypress/react'; | ||
import { FluentProvider } from '@fluentui/react-provider'; | ||
|
@@ -203,9 +204,9 @@ describe('FlatTree', () => { | |
</TreeTest>, | ||
); | ||
cy.focused().should('not.exist'); | ||
cy.document().realPress('Tab'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
So, I use |
||
cy.document().press('Tab'); | ||
cy.get('[data-testid="item1"]').should('be.focused'); | ||
cy.document().realPress('Tab'); | ||
cy.document().press('Tab'); | ||
cy.get('#action').should('be.focused'); | ||
}); | ||
describe('navigationMode="treegrid"', () => { | ||
|
@@ -241,9 +242,9 @@ describe('FlatTree', () => { | |
</TreeTest>, | ||
); | ||
cy.focused().should('not.exist'); | ||
cy.document().realPress('Tab'); | ||
cy.document().press('Tab'); | ||
cy.get('[data-testid="item1"]').should('be.focused'); | ||
cy.document().realPress('Tab'); | ||
cy.document().press('Tab'); | ||
cy.get('#action').should('be.focused').realPress('{enter}'); | ||
cy.get('[data-testid="item1__item1"]').should('not.exist'); | ||
cy.get('#action').should('be.focused').realPress('Space'); | ||
|
@@ -252,23 +253,23 @@ describe('FlatTree', () => { | |
it('should focus on first item when pressing tab key', () => { | ||
mount(<TreeTest />); | ||
cy.focused().should('not.exist'); | ||
cy.document().realPress('Tab'); | ||
cy.document().press('Tab'); | ||
cy.get('[data-testid="item1"]').should('be.focused'); | ||
}); | ||
it('should focus out of tree when pressing tab key inside tree.', () => { | ||
mount(<TreeTest />); | ||
cy.focused().should('not.exist'); | ||
cy.document().realPress('Tab'); | ||
cy.document().press('Tab'); | ||
cy.get('[data-testid="item1"]').should('be.focused'); | ||
cy.focused().realPress('Tab'); | ||
cy.focused().press('Tab'); | ||
cy.focused().should('not.exist'); | ||
}); | ||
describe('Navigation', () => { | ||
it('should move with Up/Down keys', () => { | ||
mount(<TreeTest />); | ||
cy.get('[data-testid="item1"]').focus().realPress('{downarrow}'); | ||
cy.get('[data-testid="item2"]').should('be.focused'); | ||
cy.focused().realPress('Tab').should('not.exist'); | ||
cy.focused().press('Tab').should('not.exist'); | ||
}); | ||
describe('navigationMode="treegrid"', () => { | ||
it('should move with Up/Down keys', () => { | ||
|
@@ -434,7 +435,7 @@ describe('FlatTree', () => { | |
defaultCheckedItems={['item1__item1']} | ||
/>, | ||
); | ||
cy.window().then(win => { | ||
cy.window().should(win => { | ||
expect(win.console.error).to.be.called; | ||
}); | ||
}); | ||
|
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.
hmm this overrides https://github.com/microsoft/fluentui/blob/master/scripts/cypress/src/support/component.js, which for now should be ok but if we add some substantial in future it will be missing here. can we resolve the situation to still use our base supportFile inclusion ?