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

Do not run move of new pagination and remove of old one when new one is present #1556

Merged
merged 2 commits into from
Jun 21, 2017

Conversation

karelhala
Copy link
Contributor

@karelhala karelhala commented Jun 16, 2017

After Merge of c0feb50 there was bug when new pagination was present it was removed if page/sorting has changed. So do not run removing of pagination and moving such pagination if new one is already at correct place.

Fixes #1489

@karelhala
Copy link
Contributor Author

@miq-bot add_label bug

@karelhala
Copy link
Contributor Author

@skateman this fixes the bug we were talking about, please review and check if it is OK.

@miq-bot miq-bot added the bug label Jun 16, 2017
@skateman
Copy link
Member

I was testing it and found out a strange behavior. If you remove an element (did with the last one on the last page), the whole paginator breaks and stops working...

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

LGTM

@dclarizio
Copy link

@karelhala I think the cc issue is an easy fix . . . I will try this out in the meantime.

@miq-bot
Copy link
Member

miq-bot commented Jun 21, 2017

Checked commits karelhala/manageiq-ui-classic@e1fa9be~...a362564 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

@martinpovolny martinpovolny merged commit 9e61e02 into ManageIQ:master Jun 21, 2017
@martinpovolny
Copy link
Member

Fixed --> merging.

@martinpovolny martinpovolny added this to the Sprint 64 Ending Jul 3, 2017 milestone Jun 21, 2017
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.

5 participants