Scrollbar Padding With Fixed Right Columns #265
Conversation
right fixed columns don't interfere with the natural layout
| @@ -631,6 +631,7 @@ var FixedDataTable = createReactClass({ | |||
| onColumnResize={this._onColumnResize} | |||
| onColumnReorder={onColumnReorder} | |||
| onColumnReorderMove={this._onColumnReorderMove} | |||
| showScrollbarY={showScrollbarY} | |||
There was a problem hiding this comment.
where's this value coming from? I don't see it above here in this method
There was a problem hiding this comment.
Huh, I thought I'd moved that up already.. but it's from L:641 below. It only appeared to work for me because of the adjustedWidth stuff from my prior fix. I'll push up a fix shortly.
src/FixedDataTable.js
Outdated
| @@ -817,6 +820,7 @@ var FixedDataTable = createReactClass({ | |||
|
|
|||
| _renderRows(/*number*/ offsetTop) /*object*/ { | |||
| var state = this.state; | |||
| var showScrollbarY = state.maxScrollY > 0 && state.overflowY !== 'hidden' && state.showScrollbarY !== false; | |||
There was a problem hiding this comment.
this is duplicated above. Should we pull out to a helper?
There was a problem hiding this comment.
Yeah that's a good call. Do we want that to actually be in FixedDataTableScrollHelper.js, or just as an additional method here in FixedDataTable.js?
There was a problem hiding this comment.
I think leaving a private method here is fine since it's only used in this file.
| .public/fixedDataTable/header .public/fixedDataTableCell/main { | ||
| background-color: var(--fbui-desktop-background-light); | ||
| background-image: linear-gradient(#fff, #efefef); | ||
| } | ||
|
|
||
| .public/fixedDataTable/scrollbarSpacer { | ||
| position: absolute; | ||
| z-index: 99; |
There was a problem hiding this comment.
Because the scrollbar itself overlays the elements below it, so the spacer needs to live on the same plane. Otherwise, this will happen:
|
lgtm, can you update from master and verify everythings good to go? |
|
Merged latest master in, this lgtm @wcjordan |
Description
Adds a spacer div to all rows if there is a vertical scrollbar present, resolves #260.
Motivation and Context
The spacing for the scrollbar was previously done by adjusting the width of the Scrollable Columns container. That fix no longer works with the addition of Fixed Right Columns since the Scrollable Columns are not guaranteed to be on the right edge anymore.
Now we add a spacer div on the right if the vertical scrollbar is present, and pull the Fixed Right Columns back by the width of said spacer (Scrollbar.SIZE). We also still need to keep part of the original fix and shrink the Scrollable Columns container, for situations when there is no Fixed Right Columns present.
How Has This Been Tested?
Locally
Screenshots (if appropriate):
Types of changes
Checklist: