Skip to content

fix(3901): Raise rowsVisibleChanged on setVisibleRows. #5674

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

Merged
merged 1 commit into from
Sep 20, 2016

Conversation

jeremy-j-ackso
Copy link
Contributor

Several issues and Stackoverflow threads indicate that one would expect the rowsVisibleChanged event to be emitted when a filter is applied. It seems pretty rational that rowsVisibleChanged ought to be raised by setVisibleRows(), which is what this small PR implements.

A related issue not addressed by this PR is that many of the SO threads and issues also seem to believe that a filtersChanged event should be raised after the visible rows are changed via filters. This is a common misunderstanding of how ui-grid operates in that changing filters are a different operation from changing visible rows. Since there seems to be a timing/ordering issue between when filtersChanged is raised and setVisibleRows is called this probably merits discussion before/if a change is made. Perhaps a tutorial page on events could alleviate this confusion?

#3901
#4138
http://stackoverflow.com/questions/27301690/angular-ui-grid-ng-grid-filterchanged-firing-too-frequently
http://stackoverflow.com/questions/33764946/how-to-get-filtered-data-from-paged-ui-grid

@mportuga
Copy link
Member

@jeremy-j-ackso It might make sense to add a unit test for this.

@dlgski
Copy link
Contributor

dlgski commented Sep 20, 2016

Accepting this PR. Can you update tutorial as you suggest as well?

@dlgski dlgski merged commit 2e01377 into angular-ui:master Sep 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants