Skip to content

[ZEPPELIN-1178] Tooltip: Show chart type when hovering over chart icon#1362

Closed
vensant wants to merge 2 commits intoapache:masterfrom
vensant:ZEPPELIN-1178
Closed

[ZEPPELIN-1178] Tooltip: Show chart type when hovering over chart icon#1362
vensant wants to merge 2 commits intoapache:masterfrom
vensant:ZEPPELIN-1178

Conversation

@vensant
Copy link
Contributor

@vensant vensant commented Aug 25, 2016

What is this PR for?

A usability improvement: Added tooltips for the chart icons in Zeppelin paragraphs, showing the chart types when hovering over the chart icons.

What type of PR is it?

Improvement

Todos

NA

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1178

How should this be tested?

  1. Deploy Zeppelin and navigate to zeppelin tutorial
  2. Run a paragraph and hover over the chart icon buttons to see the tooltip getting displayed.

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

…e chart types when hovering over the chart icons
@bzz
Copy link
Member

bzz commented Aug 26, 2016

@vensant thank you for contributing improvement!

With GUI changes, in my experience, adding simple screenshots to the PR itself (drag'n'drop to description) before\after usually helps to speed up the review process a lot.

CI failure seems not related, look lik networking issue on Travis side :\

[INFO] Zeppelin: web Application .......................... FAILURE [01:20 min]
[INFO] grunt-ng-annotate@0.10.0 node_modules/grunt-ng-annotate
[ERROR] npm ERR! argv "/home/travis/build/apache/zeppelin/zeppelin-web/node/node" "/home/travis/build/apache/zeppelin/zeppelin-web/node/node_modules/npm/bin/npm-cli.js" "install" "--color=false"
[ERROR] npm ERR! node v0.12.13
[ERROR] npm ERR! npm  v2.15.0
[ERROR] 
[ERROR] npm ERR! Callback called more than once.
[ERROR] npm ERR! 
[ERROR] npm ERR! If you need help, you may report this error at:
[ERROR] npm ERR!     <https://github.com/npm/npm/issues>
[ERROR] 
[ERROR] npm ERR! Please include the following file with any support request:
[ERROR] npm ERR!     /home/travis/build/apache/zeppelin/zeppelin-web/npm-debug.log

@corneadoug
Copy link
Contributor

Tested, LGTM
However maybe the tooltip names could be changed to something more simple:

  • Multi Bar Chart -> Bar Chart
  • Stacked Area Chart -> Area Chart
  • Line With Focus Chart -> Line Chart

@Leemoonsoo
Copy link
Member

+1 for @corneadoug 's comment. Shorter name is actually more clear and easier to understand.

@vensant
Copy link
Contributor Author

vensant commented Aug 26, 2016

Hi all,

I will change the tool tip names as suggested.

Thank you all

Regards,
Venkat

On 26 Aug 2016 7:57 am, "Lee moon soo" notifications@github.com wrote:

+1 for @corneadoug https://github.com/corneadoug 's comment. Shorter
name is actually more clear and easier to understand.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1362 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALgNdQZ6-GggaW1xEpI4HeHAPhhR5Wq6ks5qjk8NgaJpZM4Js5xw
.

@prabhjyotsingh
Copy link
Contributor

+1 for @corneadoug, rest LGTM!

@vensant
Copy link
Contributor Author

vensant commented Aug 26, 2016

Code changes done as per the review comment.

@corneadoug
Copy link
Contributor

@vensant Thank you! Merging if there is no more discussions

@vinayshukla
Copy link
Contributor

Thanks @vensant

@asfgit asfgit closed this in b7f918a Aug 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants