Add left and right shadows to indicate that the table is horizontally scrollable #58
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can you include some screenshots? |
KamranAsif
reviewed
Oct 6, 2016
| 'fixedDataTableRowLayout/fixedColumnsDivider': left > 0, | ||
| 'fixedDataTableRowLayout/columnsShadow': this.props.scrollLeft > 0, | ||
| 'public/fixedDataTableRow/fixedColumnsDivider': left > 0, | ||
| 'public/fixedDataTableRow/columnsShadow': this.props.scrollLeft > 0, |
| 'fixedDataTableRowLayout/columnsShadow': true, | ||
| 'fixedDataTableRowLayout/columnsRightShadow': true, | ||
| 'public/fixedDataTableRow/columnsShadow': true, | ||
| 'public/fixedDataTableRow/columnsRightShadow': true, |
There was a problem hiding this comment.
I think you can just do cx('class1', 'class2', ...);
| }, | ||
|
|
||
| _renderColumnsRightShadow(/*number*/ totalWidth) /*?object*/ { | ||
| if (this.props.scrollLeft + this.props.width + 0.5 < totalWidth) { |
There was a problem hiding this comment.
What is 0.5 magic number here?
There was a problem hiding this comment.
Changed to use 'Math.ceil' to roundup the value so it's more readable. There is a chance the shadow doesn't show nicely at an edgy case otherwise.
| .public/fixedDataTableRow/columnsRightShadow { | ||
| -webkit-transform: rotate(180deg); | ||
| -moz-transform: rotate(180deg); | ||
| -ms-transform: rotate(180deg); |
There was a problem hiding this comment.
We have postcss as part of the build step, so you don't need to manually include all prefixes
There was a problem hiding this comment.
Removed unnecessary prefixes.
7 tasks
KamranAsif
suggested changes
Oct 7, 2016
| left: left, | ||
| height: this.props.height | ||
| }; | ||
| console.log("left shadow: ", className); |
|
LGTM, thanks! |
KamranAsif
approved these changes
Oct 7, 2016
|
Screenshots added
|
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added shadows to the table on both left and right sides to indicate the table is horizontally scrollable.
Description
When a table has many columns, horizontal scrollbar shows up to allow users to scroll horizontally to the right or the left to view columns on the two ends. The change adds shadows to one or both sides to indicate if there are columns not being exposed thus the table can be scrolled to view them.
Motivation and Context
Due to user experience tests, horizontal scrollbars are not clear enough to users that the they can scroll horizontally to view columns on the sides that are currently hidden. Having shadows makes it more clear and users can easily understand if they have hidden columns that they need to scroll to view.
How Has This Been Tested?
Very minor JavaScript code change and some simply CSS style change.
Manual testing only. No automated test. It has been tested in IE11+ (Win10), Edge(Win10), Chrome (MacOS), Safari(MacOS), Firefox(MacOS).
No impact on other area of the code is expected.
Types of changes
Checklist: