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 hidden x-scrollbar #210

Merged
merged 1 commit into from
Jul 17, 2019
Merged

Conversation

dpordomingo
Copy link
Contributor

@dpordomingo dpordomingo commented Jul 12, 2019

fix #184
supersedes #209
related to #196 (comment)

Superset calculates the height of the filtered table with javascript, using hardcoded heights for tabs [1] and buttons [2], so when there is more than one line of tabs, the filtered table does not fit in the available space, appearing a new outer scroll (as it can be seen in the screenshot below) as described by #184.

With this PR, the filtered table is positioned using absolute coordinates, leaving space for all the components in the panel, no matter how much lines of tabs are opened.

If this PR is approved, the SQL Lab would be like this 👇

image

instead of how it was in the previous version without branding: v0.3.0 👇

image

Signed-off-by: David Pordomingo <[email protected]>
@dpordomingo dpordomingo added the bug Something isn't working label Jul 12, 2019
@dpordomingo dpordomingo self-assigned this Jul 12, 2019
@dpordomingo
Copy link
Contributor Author

Only tested in Chrome-Linux; could you please try in other supported browsers?

@dpordomingo dpordomingo requested a review from a team July 12, 2019 13:32
@ricardobaeta ricardobaeta self-requested a review July 12, 2019 13:37
Copy link
Contributor

@ricardobaeta ricardobaeta left a comment

Choose a reason for hiding this comment

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

✨ Thank you!

@carlosms
Copy link
Contributor

To add more context: the extra scrollbar that I reported is there even if there is only one row of tabs, since the hardcoded heights do not match the new css.

Which is a different problem to having more than one row of tabs.

Testing this locally, it fixes both problems.

@dpordomingo should this be contributed upstream? The problem will happen with more than one row of tabs, and they might also prefer to have css sizing instead of a hardcoded value that can be more prone to errors.

@dpordomingo
Copy link
Contributor Author

Yes, this could be contributed into superset.
Do you agree on merging, and postpone the contribution later?
As soon as it's accepted, I'll apply it here.

@carlosms
Copy link
Contributor

ok.

But let's first test it with firefox too at least.

@dpordomingo
Copy link
Contributor Author

I'd say that it's also ok in Firefox 👇
image

@dpordomingo dpordomingo merged commit b6306b6 into src-d:master Jul 17, 2019
@dpordomingo dpordomingo deleted the fix-sql-panel branch July 21, 2019 17:15
@smacker smacker mentioned this pull request Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extra outer scrollbar in SQL lab results
3 participants