-
Notifications
You must be signed in to change notification settings - Fork 2.8k
ZEPPELIN-491: Giving fixed height to container when result type is text #519
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
Conversation
|
Also I've removed "loadResultType" function calls, as it did not had any respective loadResultType declaration or implementation. |
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.
empty else {} is a bit odd?
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.
I picked template function from $scope.renderHtml; fixed both $scope.renderHtml and $scope.renderText
93c8f38 to
3f14de7
Compare
3f14de7 to
dc96ed6
Compare
|
Looks good. Can we get some attention from committers. Thanks! |
|
wow you are building Zeppelin from within Zeppelin ;) |
| ng-click="scrollParagraphDown()" ></div> | ||
| <div id="p{{paragraph.id}}_text" | ||
| style="max-height: {{paragraph.config.graph.height}}px; overflow: auto" | ||
| ng-scroll="someFunction()" |
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.
What is that line for? I didn't see any change when trying with and without.
Not sure if its necessary since someFunction doesn't exist anywhere
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.
Totally missed to remove this.
|
Could you use the same tooltip as everywhere else in zeppelin for those two buttons? |
|
@corneadoug Thanks for the review. I was just experimenting with look and feel of those two buttons, but what you recommend makes more sense. |
e7e2f90 to
fc95003
Compare
- have standard toooltip for Follow output and scroll top
fc95003 to
287fff8
Compare
|
@corneadoug does this looks ok? |
|
Yes, LGTM |
|
merging if no further discussion |


In current behaviour we have a fixed height for result container that contains either graph or table, but if type is text then the container expands to infinitely.
When type is text; then this is mostly logs, so we can have scroll around it and have fixed height for this container.
Before

After
