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

T1261532: DataGrid - FocusedRowChanged event isn't raised when the push API is used to remove the last row #28687

Open
wants to merge 11 commits into
base: 25_1
Choose a base branch
from

Conversation

Raushen
Copy link
Contributor

@Raushen Raushen commented Jan 8, 2025

No description provided.

@Raushen Raushen added the 25_1 label Jan 8, 2025
@Raushen Raushen self-assigned this Jan 8, 2025
@Raushen Raushen requested a review from a team as a code owner January 8, 2025 18:07
Comment on lines 245 to 247
await t
.expect(Selector('#otherContainer').innerText)
.eql('Success');
Copy link
Contributor

@Tucchhaa Tucchhaa Jan 20, 2025

Choose a reason for hiding this comment

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

I feel strange about the checking the text of #otherContainer. Is it possible to use this instead of setting text in onFocusedRowChanged?

Suggested change
await t
.expect(Selector('#otherContainer').innerText)
.eql('Success');
const key = await grid.option('focusedRowKey');
const index = await grid.option('focusedRowIndex');
await t.expect(key === null && index === -1).ok();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TestCafe is designed to work with markup, and in our tests, we use the Page Object Model for the DataGrid, which operates with a markup (CCS classes and DOM elements) and does not have a method for retrieving options from the business object. It seems that transforming the state into markup and verifying it using TestCafe is the most appropriate and natural approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

the current test implementation does its job, but it just feels unnaturally to me to do it like that. If using grid.option() will also do the job, then I suggest to use it 🙂

Copy link
Contributor Author

@Raushen Raushen Jan 20, 2025

Choose a reason for hiding this comment

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

Yep, finally I see, thank you! The correct check looks slightly different)
The whole expression should be wrapped to the "except" method so the expression could calculate continuously, not only once while a declaration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants