-
Notifications
You must be signed in to change notification settings - Fork 14k
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
Making time table viz scrollable #3808
Making time table viz scrollable #3808
Conversation
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.
Is there place where we change slice.
@@ -36,6 +36,7 @@ FormattedNumber.propTypes = { | |||
|
|||
function viz(slice, payload) { | |||
slice.container.css('height', slice.height()); | |||
slice.container.css('overflow', 'scroll'); |
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.
why not set this style in css file, the container has a class...
Coverage remained the same at 71.445% when pulling a737c1613060c8aad67062128ba22570526ccbb5 on michellethomas:timetable_viz_scroll into 4fa1f0a on apache:master. |
1 similar comment
Coverage remained the same at 71.445% when pulling a737c1613060c8aad67062128ba22570526ccbb5 on michellethomas:timetable_viz_scroll into 4fa1f0a on apache:master. |
a737c16
to
9ff9a95
Compare
I'm not sure what the padding: 10px was set for, but it didn't seem to be getting applied correctly and it looks like too much padding. |
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.
LGTM
Previously time table viz didn't scroll when given a large number of results.
Fixes #3802