[SQL Lab] Remove double scrollbar from the result tab #7083
Conversation
|
@xtinec I know you touched this layout recently, mind reviewing this? I assume this was tested on both FF and Chrome. Screenshots in layout-related issues/PR are always welcomed! |
| }; | ||
|
|
||
| const SEARCH_HEIGHT = 46; | ||
| const SEARCH_HEIGHT = 186; |
There was a problem hiding this comment.
hmm, I think the hardcode here is the height of the ResultSetControl div. It appears to be 46 pixels in both Chrome and FF. Are we sure about 186 here?
There was a problem hiding this comment.
as it was, the result table was going out of the screen (last row never visible, unless you would scroll the outer panel)
I'm generally against magic numbers in the code though, so we should probably find a proper solution for it (ie calculating the sibling heights after rendering) :)
| ref="Table" | ||
| headerHeight={headerHeight} | ||
| height={height - 2} | ||
| height={height - 20} |
|
Also, I have a big-ish layout refactoring PR that addresses this and the issues on Firefox. Coming out in the next day or two. |
|
@xtinec yes, let's go over this after your PR - i believe this is gonna be superfluous then :) thanks Christine! |
|
@enricoberti yep, I think #7102 supersedes it. 🙌 |
|
@xtinec on the last master I still have the double scrollbar though - it seems that the outer panel it too tall. Do you want me to have a look? Thanks!
|
|
@enricoberti Yeah, I'm seeing it as well. Trying to figure out why. I noticed it only happens when there is a horizontal scroll bar, meaning the height of the react virtualized table is off by the scrollbar height. Strangely, I accounted for this in the PR. Not sure what's causing it. 🤔 Any thoughts? |
|
@enricoberti okay, I just figured it out. 🙂 PR out to you shortly. 🙌 |
|
Thanks @xtinec! Closing this PR as superseded by yours :) |
3 participants
closes #7081