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: pagination controls should not scroll horizontally for saved search #50764

Conversation

mbondyra
Copy link
Contributor

@mbondyra mbondyra commented Nov 15, 2019

Summary

Bug: The pagination controls in the header and footer of the saved search widget scroll along with the table.

bug

This PR fixes the problem - the controls keep their position to the right corner of the widget.

fixed

Fixes #49757

PS. I use negative margin to include controls from the bottom above the scroll so the design doesn't change. If I haven't, it would look like here:
image

Another option to 'stick' the position of the controls would be to use JS, but I feel it's too much for this fix. Let me know what you think!

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@mbondyra mbondyra changed the title Fix: pagination controls should not scroll vertically for saved search Fix: pagination controls should not scroll horizontally for saved search Nov 15, 2019
@mbondyra mbondyra added release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.6.0 v8.0.0 labels Nov 15, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@mbondyra mbondyra force-pushed the IS_-49757-fix_saved_search_pag_controls_moves_vertical_scrolling branch from e4812fd to ee4bdb8 Compare November 15, 2019 11:18
@mbondyra mbondyra marked this pull request as ready for review November 15, 2019 11:24
@mbondyra mbondyra requested a review from a team as a code owner November 15, 2019 11:24
@mbondyra mbondyra requested review from timroes, flash1293 and a team November 15, 2019 11:25
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@majagrubic
Copy link
Contributor

majagrubic commented Nov 15, 2019

Not sure if it is a result of this PR or not, but the pagination overflows the last row in the widget:
Screenshot 2019-11-15 at 15 41 35

I haven't tested on master, but without the introduced padding, the pagination controls are back in place:
Screenshot 2019-11-15 at 15 44 01

Screenshot 2019-11-15 at 15 45 19

@mbondyra
Copy link
Contributor Author

Thank you @majagrubic, it's fixed now!

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Thanks @mbondyra ! Can you also update some of these other hard-coded pixel values to be EUI variables/calculations of these variables? This way our scale stays consistent in case we change the base. I gave suggestions on how to convert them.

@@ -62,6 +62,16 @@ doc-table {
margin: $euiSizeXS $euiSizeXS 0;
}

.kbnDocTable__bar--footer {
position: relative;
margin: -48px $euiSizeXS 0 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
margin: -48px $euiSizeXS 0 0;
margin: -($euiSize * 3) $euiSizeXS 0 0;



.kbnDocTable__padBottom {
padding-bottom: 32px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
padding-bottom: 32px;
padding-bottom: $euiSizeXL;

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Thanks!

margin: -($euiSize * 3) $euiSizeXS 0;
}


Copy link
Contributor

Choose a reason for hiding this comment

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

⚡️ empty line

@mbondyra mbondyra force-pushed the IS_-49757-fix_saved_search_pag_controls_moves_vertical_scrolling branch from 2dfcd91 to 3f0a162 Compare November 15, 2019 17:11
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@flash1293
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested and works as expected. LGTM 👍

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mbondyra mbondyra merged commit 0fa36f8 into elastic:master Nov 18, 2019
@mbondyra mbondyra deleted the IS_-49757-fix_saved_search_pag_controls_moves_vertical_scrolling branch November 18, 2019 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saved search pagination moves on vertical scrolling
5 participants