Issue 216 - Scrollbar alignment#247
Conversation
| "@storybook/cli": "^4.0.2", | ||
| "@storybook/react": "^4.0.2", | ||
| "@storybook/cli": "^4.0.7", | ||
| "@storybook/react": "^4.0.7", |
There was a problem hiding this comment.
Having issues running storybook locally, updating fixes the issue -- local dep of react needs to be updated
| resolve(width); | ||
| }, 0); | ||
| }); | ||
| }; No newline at end of file |
There was a problem hiding this comment.
Create a phantom div with a child to calculate the width of the scrollbar -- if the scrollbar disappears, it will have 0 width
There was a problem hiding this comment.
(Promise polyfill for IE11 is managed automatically by Babel)
There was a problem hiding this comment.
Looks good - I had a few questions about if this shouldn't just be a React component - had a small discussion with @Marc-Andre-Rivet and came to the conclusion that because this is just a calculation, it doesn't necessarily need to be a React component.
|
|
||
| this.updateStylesheet(); | ||
|
|
||
| getScrollbarWidth().then((scrollbarWidth: number) => this.setState({ scrollbarWidth })); |
There was a problem hiding this comment.
Re-evaluate scrollbar width on resize (to cover zoom case) and update state
| const fragmentStyles: (CSSProperties | undefined)[][] = [ | ||
| [ | ||
| undefined, | ||
| { marginRight: this.state.scrollbarWidth } |
There was a problem hiding this comment.
Add a margin to the fixed rows
|
Hi, This Abhishek from Invesco. Just wanted to know if this issue is fixed and if not is there any workaround from styling perspective included in our code. |
|
@vedulaabhi This PR should fix the issue in all known scenarios on all Mac and Windows browsers supported by the table. No additional workaround is necessary to make the fix work. |
|
@Marc-Andre-Rivet will you be including this in the next release and if so can you pls let me know when we can expect the release as this is really important for our ispace app. |
|
@vedulaabhi Once the review goes through this will be released as 3.1.7 -- this is the next release :) |
valentijnnieman
left a comment
There was a problem hiding this comment.
A small nitpick, other than that it LGTM.
| key={columnIndex} | ||
| ref={`r${rowIndex}c${columnIndex}`} | ||
| ref={`r${rowIndex}c${columnIndex}`} | ||
| style={fragmentStyles[rowIndex][columnIndex]} |
There was a problem hiding this comment.
Small nitpick about indentation here. 🐱
| resolve(width); | ||
| }, 0); | ||
| }); | ||
| }; No newline at end of file |
There was a problem hiding this comment.
Looks good - I had a few questions about if this shouldn't just be a React component - had a small discussion with @Marc-Andre-Rivet and came to the conclusion that because this is just a calculation, it doesn't necessarily need to be a React component.
Fixes #216, #203.
After a pretty big false start, using a "phantom" div we can calculate the width of the vertical scrollbar and apply that width as an extra margin for the fixed rows section.. allowing scrolling to stay aligned throughout.
With disappearing scrollbars:
With always visible scrollbars:
