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

fix(esf): reset forOf startIndex on esf columnChange #14125

Merged
merged 4 commits into from
Apr 23, 2024

Conversation

onlyexeption
Copy link
Contributor

Closes #14043

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@@ -229,6 +229,10 @@ export class IgxExcelStyleSearchComponent implements AfterViewInit, OnDestroy {
});
esf.columnChange.pipe(takeUntil(this.destroy$)).subscribe(() => {
this.virtDir?.resetScrollPosition();

if (this.virtDir) {
this.virtDir.state.startIndex = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why this is necessary. Is perhaps the scrollbar already hidden at the time you are trying to reset it?
I also cannot reproduce the issue as is described in the original issue steps in 17.1.x:
filtring

Am I perhaps missing something or are steps slightly different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can reproduce the issue in the ESF templates sample below.
Since we're reusing the IgxGridExcelStyleFilteringComponent and we do not reset the startIndex anymore (from #13688), it doesnt reset when changing the column.

screen-capture.webm

Notice how the startIndex of the second column is 14.

Copy link
Contributor

@MayaKirova MayaKirova left a comment

Choose a reason for hiding this comment

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

I think it would be best to add a test specifically for this scenario so it does not get regressed anymore.

@teodosiah teodosiah added 💥 status: in-test PRs currently being tested ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification 💥 status: in-test PRs currently being tested labels Apr 22, 2024
@gedinakova gedinakova merged commit 2d36240 into 17.1.x Apr 23, 2024
4 checks passed
@gedinakova gedinakova deleted the ibarakov/fix-14043-17.1.x branch April 23, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grid: excel-style-filtering version: 17.1.x ✅ status: verified Applies to PRs that have passed manual verification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants