Skip to content

Conversation

@jasonxh
Copy link
Contributor

@jasonxh jasonxh commented Jun 7, 2016

What is this PR for?

This PR fixes a few minor issues from the recent introduction of Handsontable for table rendering (#858):

  • Render up to 5 digits after decimal point instead of always rounding to integers
  • Allow visual selection of table cells (for copy)
  • Default to text renderer instead of numeric renderer

What type of PR is it?

Bug Fix, Improvement

Todos

What is the Jira issue?

How should this be tested?

Output some rows with floating point numbers and render them in a table.

Screenshots (if appropriate)

Questions:

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

* Render up to 5 digits after decimal point
* Allow visual selection of table cells
* Default to text renderer
@corneadoug
Copy link
Contributor

@jasonxh Thanks for the fix
Next time if possible, please add a simple code example for testing, like:

print(s"""%table
name\tsize
sun\t127.6
moon\t0.6458
gerard\t45.234856803""")

It can save a lot of time to the reviewer :)

I tested and the decimal formating is working great, HTML and string rendering is still good too.

Regarding the selection cell for copy, I would advise to use both:

fragmentSelection: true,
disableVisualSelection: true,

This would remove the cell selection (with the border), and allow selection of the table text visually.
screen shot 2016-06-08 at 2 36 59 pm

So except for that small configuration change, LGTM

@jasonxh
Copy link
Contributor Author

jasonxh commented Jun 8, 2016

Thanks @corneadoug . I've updated the PR with your suggestions. I'll remember to paste a code snippet next time. :)

@corneadoug
Copy link
Contributor

LGTM
Anybody's welcome to test it too

@r-kamath
Copy link
Member

r-kamath commented Jun 8, 2016

@corneadoug :)
@jasonxh Thanks for the fix. LGTM

@corneadoug
Copy link
Contributor

Merging if there is no more discussions

@corneadoug
Copy link
Contributor

@jasonxh I can't find you somehow on the JIRA user id.
Could you give it to me?

@jasonxh
Copy link
Contributor Author

jasonxh commented Jun 9, 2016

@corneadoug it's the same handle as github, jasonxh

@corneadoug
Copy link
Contributor

@jasonxh can you try to assign yourself to those two JIRA issues? somehow I can't do it

@corneadoug
Copy link
Contributor

@jasonxh I managed to do it, thanks

@gauravkumar37
Copy link

gauravkumar37 commented Jun 19, 2016

Regarding selecting data from the table (ZEPPELIN-954), it works but only the first time. I have tried using both Firefox and Chrome (latest versions).
I am using %sql to output a table from spark sql. It works when I refresh the page and the data is pre-populated from the previous run. After I run the query again and fresh data is rendered, selecting text doesn't work.
Also, the text editor becomes very sluggish after running any query, until I refresh the page.

@jasonxh
Copy link
Contributor Author

jasonxh commented Jun 21, 2016

@gauravkumar37 I noticed the same thing. There's some cleanup issue regarding handsontable. I've reopened [ZEPPELIN-954] and I'm working on a fix.

@corneadoug
Copy link
Contributor

Handsontable isn't properly instantiated, it's creating a lot of them

@gauravkumar37
Copy link

@corneadoug I think probably that's the reason the UI gets sluggish after a while.

@jasonxh jasonxh deleted the hao/render-table branch June 27, 2016 16:42
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.

4 participants