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

Page navigation should not be included in the query results table #322

Closed
madalincm opened this issue Jan 26, 2018 · 12 comments
Closed

Page navigation should not be included in the query results table #322

madalincm opened this issue Jan 26, 2018 · 12 comments
Assignees
Milestone

Comments

@madalincm
Copy link

Steps to Reproduce:

  1. Open this query: https://pipeline-sql.stage.mozaws.net/queries/273/source
  2. Scroll down the page and observe the scroll bar and the page navigation positioning.

Expected results:

The page navigation is displayed outside the table. When the users uses the scroll bar the navigation is keeping it's position.

Actual results:

The page navigation is displayed inside the table. When the user scrolls to left the navigation is no longer displayed.

Notes/Issues:

Build identifier: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Verified in FF58(Win7). Issue can be reproduced in STMO-stage

uichangesscrollpagenavigation

@rafrombrc rafrombrc added this to the 14 milestone Feb 21, 2018
@madalincm
Copy link
Author

The query results are now truncated so I'm not able to test this properly. I have logged #342

@madalincm
Copy link
Author

#342 Is no longer reproducing but the navigation is still inside the query results table. If the user scrolls to the right the navigation is no longer displayed.

@alison985
Copy link

@jezdez Would you agree this is fixed by centering the pagination navigation? Centering pagination occurred in #320 with PR #413. Just want to confirm that fixing that then fixes this.

@jezdez
Copy link

jezdez commented Jun 5, 2018

I don't think if centering the pagination (#320, #413) fixes this issue, as this it's about moving the paginator outside the div with the query results to prevent the paginator from vanishing in the first place.

Since this is obviously an issue from upstream, I suggest to open a ticket upstream and submitting a PR there, no need to create extra work to backport it later.

@alison985
Copy link

see getredash#2584

@alison985 alison985 self-assigned this Jun 12, 2018
@alison985 alison985 added blocked and removed ready labels Jul 4, 2018
@rafrombrc rafrombrc modified the milestones: 15, 16 Jul 11, 2018
@alison985 alison985 removed the blocked label Jul 16, 2018
@alison985
Copy link

merged upstream

@jezdez
Copy link

jezdez commented Jul 16, 2018

Huzzah!

@jezdez jezdez closed this as completed Jul 16, 2018
@madalincm
Copy link
Author

Verified in FF61(Win7) in stmo-stage
The issue seems partially fixed. At this moment the page navigation is not affected by the horizontal scroll but when using the vertical scroll the navigation can be hided.
Please view this screecast:
navigationveriticalscroll

Reopening the issue.

@madalincm madalincm reopened this Jul 31, 2018
@alison985
Copy link

alison985 commented Aug 12, 2018

I've spent about an hour playing with this, but there is a CSS issue getting in the way of what I think Madalin is asking for. Namely: "The overflow property only works for block elements with a specified height." So I can add "overflow-y: scroll; display: block;" to the dynamic-table-container but to get it to work I have to add a pixel based height such as "height: 100px;" which makes the results table not responsive to monitor size. I have looked into the flex- components of CSS but they seem to be a) relative and b) not to work in this setting. @jezdez Can we close this since the navigation is now accessible again?

@jezdez
Copy link

jezdez commented Aug 14, 2018

@alison985 Yeah, let's close this ticket since it was partially fixed.

@madalincm Since this part of the Redash UI is heavily owned by the Redash team we need to file a bug upstream and see what their opinion on the hiding when vertically scrolling is. I'm afraid that we're running in circles when we only discuss this on the Mozilla side. Could you open a ticket upstream please?

@madalincm
Copy link
Author

@jezdez I have logged the issue upstream getredash#2749

@jezdez
Copy link

jezdez commented Aug 16, 2018

@madalincm Thanks!

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

No branches or pull requests

4 participants