-
Notifications
You must be signed in to change notification settings - Fork 2.9k
ScrollablePane: Optimization on how component works #4491
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
Changes from 45 commits
2ae8032
2534902
351b509
d7befce
2e6f1eb
4cc5960
30b8c79
de0424b
0fe99c9
92d4db5
de3070c
b72e391
e4ab481
7fe6b8c
7992359
0397c02
0082fb4
a143c76
ca65251
b4a7820
697abf8
9e012ff
5d894de
0ff1aec
57faff0
8741d7c
7d86387
0b6badb
3799d87
a7aeb07
aac9e53
0080b46
0313824
28709a7
78a298f
59f2ab7
0ef10da
c55aad9
b8aab4c
489d246
ac905dc
474bf81
1fa5a01
e04d2a7
00a8b44
b49fa90
c5b0ef3
f37feb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import { | |
|
|
||
| const GlobalClassNames = { | ||
| root: 'ms-ScrollablePane', | ||
| contentContainer: 'ms-ScrollablePane--contentContainer' | ||
| }; | ||
|
|
||
| export const getStyles = ( | ||
|
|
@@ -24,19 +25,39 @@ export const getStyles = ( | |
| position: 'absolute', | ||
| pointerEvents: 'auto', | ||
| width: '100%', | ||
| zIndex: ZIndexes.ScrollablePane, | ||
| overflowY: 'hidden', | ||
| overflowX: 'auto' | ||
| }; | ||
|
|
||
| const maxHeightStyles: IStyle = { | ||
| height: 'inherit', | ||
| maxHeight: 'inherit', | ||
| zIndex: ZIndexes.ScrollablePane | ||
| }; | ||
|
|
||
| return ({ | ||
| root: [ | ||
| classNames.root, | ||
| { | ||
| WebkitOverflowScrolling: 'touch', | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a lowercase |
||
| position: 'absolute', | ||
| top: 0, | ||
| right: 0, | ||
| bottom: 0, | ||
| left: 0 | ||
| }, | ||
| maxHeightStyles, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make |
||
| className | ||
| ], | ||
| contentContainer: [ | ||
| classNames.contentContainer, | ||
| { | ||
| overflowY: 'auto', | ||
| maxHeight: 'inherit', | ||
| height: 'inherit', | ||
| position: 'relative', | ||
| WebkitOverflowScrolling: 'touch' | ||
| }, | ||
| className | ||
| maxHeightStyles | ||
| ], | ||
| stickyAbove: [ | ||
| { | ||
|
|
@@ -59,6 +80,12 @@ export const getStyles = ( | |
| } | ||
| }, | ||
| AboveAndBelowStyles | ||
| ], | ||
| stickyBelowItems: [ | ||
| { | ||
| bottom: 0 | ||
| }, | ||
| AboveAndBelowStyles | ||
| ] | ||
| }); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| - Don't use Sticky on elements with margin top or bottom | ||
| - Don't use Sticky on elements with `margin-top` or `margin-bottom` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| - Ensure that a parent component has a CSS height, or max-height attribute set (and any intermediary containers have an inherit, or explicit height/max-height set). | ||
| - ScrollablePane uses `position: absolute`. Ensure that the parent element has an explicit height, or has space already allocated for ScrollablePane (e.g: flexbox). | ||
| - Use Sticky component on block level elements | ||
| - Sticky component are ideally section headers and/or footers | ||
| - Sticky component are ideally section headers and/or footers | ||
| - Ensure that the total height of Sticky components do not exceed the height of the ScrollablePane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get rid of the
maxHeightStylesaltogether now?