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

[Lens] pagination filtering bug workaround #139156

Closed

Conversation

drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented Aug 19, 2022

Summary

fix #139126

This is a quick workaround for the EUI bug where the EuiDataGrid treats cell actions differently when they aren't on the first page. We should still nail down the details on this, but this PR is available if we need it for 8.4.

This isn't perfect since the user loses their place in the table when they filter by a cell value and then remove that filter.

Screen.Recording.2022-08-18.at.8.11.21.PM.mov

Checklist

Delete any items that are not applicable to this PR.

@drewdaemon drewdaemon added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:skip Skip the PR/issue when compiling release notes Feature:Lens labels Aug 19, 2022
@drewdaemon drewdaemon changed the title [Lens] pagination filtering bug fix [Lens] pagination filtering bug workaround Aug 19, 2022
@flash1293 flash1293 added release_note:fix backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed release_note:skip Skip the PR/issue when compiling release notes labels Aug 19, 2022
@flash1293
Copy link
Contributor

Good catch @andrewctate . I looked into applying the same fix for the agg based table (it's happening there as well), but afterwards I noticed that it's not sufficient. The problem is in general that the pagination doesn't follow if the data changes - this can also happen for other reasons than a filter click:

  • Changing time range
  • Filter click in another table on the same dashboard
  • ...

So what we really need to do is to catch the case of a "too advanced page" before it crashes and reset. I will prepare a separate PR for that.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 19, 2022

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #2 / TableVisBasic should sort rows by column and pass the sorted rows for consumers

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.2MB 1.2MB +46.0B
visTypeTable 15.7KB 15.8KB +46.0B
total +92.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@drewdaemon
Copy link
Contributor Author

closing in favor of #139160

@drewdaemon drewdaemon closed this Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Lens release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filtering from second page of lens and agg based table breaks the table viz.
4 participants