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

Fixed EuiInMemoryTable content render on changing pagination #4714

Merged
merged 4 commits into from
Apr 19, 2021

Conversation

hetanthakkar
Copy link
Contributor

@hetanthakkar hetanthakkar commented Apr 15, 2021

Summary

Fixed issue #3231 Disabling pagination will render the whole content in the table and enabling it will again allow pagination in the table

Checklist

@kibanamachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@hetanthakkar
Copy link
Contributor Author

@chandlerprall @cchaos Could you please review!

@thompsongl
Copy link
Contributor

Thanks for the PR, @hetanthakkar1!
We'll review this soon. In the meantime, can you please check off the tasks you've completed in the "Checklist?" If any tasks don't apply to this PR, you can strike-through the item. Having an accurate Checklist helps us see what's been completed.

@hetanthakkar
Copy link
Contributor Author

@thompsongl Sure! Thank you

@hetanthakkar
Copy link
Contributor Author

@cchaos @chandlerprall @thompsongl Could you please review!

@thompsongl
Copy link
Contributor

jenkins test this

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Code changes LGTM; thanks, @hetanthakkar1

Just some changelog clean up.

CHANGELOG.md Outdated Show resolved Hide resolved
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4714/

@hetanthakkar
Copy link
Contributor Author

@thompsongl I have made the necessary changes. Could you please review!

@hetanthakkar
Copy link
Contributor Author

hetanthakkar commented Apr 19, 2021

@thompsongl I was just curious and wondering that you maintainers after reviewing our PR usually test them by writing Jenkin, Test this. But if we contributors are also given the freedom to do that I think the process can speed up and save time as we will be executing that command after pushing the code and you won't have to write that !? Obviously, you guys would have thought about this but I just wanted to know why it can't happen that way

@cchaos
Copy link
Contributor

cchaos commented Apr 19, 2021

Hi @hetanthakkar1 We understand it's a slow manual process for reviewing community PR's and we appreciate your patience with us as we juggle maintenance of the repo alongside our other Elastic duties. The CI builds, and triggering them, are completely permissions based and managed by our internal infrastructure team. Since community members don't have access to the builds, they also can't trigger them. Sorry, but it's not part of our control on that side.

@hetanthakkar
Copy link
Contributor Author

@cchaos I get it. I didn't know these CI builds are something that is very permission-based and important. I thought CI build is like test commands! I really appreciate your reviews and have learnt a lot while contributing. Thank you!

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4714/

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

Successfully merging this pull request may close these issues.

4 participants