Skip to content

[ZEPPELIN-954] Fix table cell selection problem on second run by properly destroying hot.#1059

Closed
jasonxh wants to merge 3 commits into
apache:masterfrom
optimizely:hao/hot-fix
Closed

[ZEPPELIN-954] Fix table cell selection problem on second run by properly destroying hot.#1059
jasonxh wants to merge 3 commits into
apache:masterfrom
optimizely:hao/hot-fix

Conversation

@jasonxh
Copy link
Copy Markdown
Contributor

@jasonxh jasonxh commented Jun 21, 2016

What is this PR for?

  • Fix table cell selection problem on second run by properly destroying hot.
  • Also make cells readonly. Previously one were able to paste into them.

What type of PR is it?

[Bug Fix]

Todos

What is the Jira issue?

How should this be tested?

Execute the following paragraph multiple times, and verify the table cells are still selectable.

%sh
echo %table
echo -e "col1\tcol2\tcol3"
echo -e "1\t2.1\tabcdefg"

Also try to paste anything into a cell to no avail.

Screenshots (if appropriate)

Questions:

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

Also make cells readonly. Previously one were able to paste into them.
@jasonxh jasonxh changed the title Fix table cell selection problem on second run by properly destroying hot. [ZEPPELIN-954] Fix table cell selection problem on second run by properly destroying hot. Jun 21, 2016
@minahlee
Copy link
Copy Markdown
Member

Tested both cases, looks good to me 👍

@minahlee
Copy link
Copy Markdown
Member

\cc @corneadoug for review

var renderTable = function() {
var height = $scope.paragraph.config.graph.height;
angular.element('#p' + $scope.paragraph.id + '_table').css('height', height);
var container = angular.element('#p' + $scope.paragraph.id + '_table').css('height', height).get(0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works great, but could we keep those two actions separated?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i figured we could just start using the return value and save an extra lookup for the same selector. pls let me know if you have strong preference and i can revert this line.

@corneadoug
Copy link
Copy Markdown
Contributor

Tested, No more handsontable duplicates.
Was there no other way to just update the data instead of destroying the table and creating a new one?

@jasonxh
Copy link
Copy Markdown
Contributor Author

jasonxh commented Jun 23, 2016

@corneadoug good suggestion. i updated the pr to reuse the table when possible.

@minahlee
Copy link
Copy Markdown
Member

Merge it there is no more discussion

@asfgit asfgit closed this in e1b3847 Jun 25, 2016
asfgit pushed a commit that referenced this pull request Jun 25, 2016
…erly destroying hot.

### What is this PR for?
* Fix table cell selection problem on second run by properly destroying hot.
* Also make cells readonly. Previously one were able to paste into them.

### What type of PR is it?
[Bug Fix]

### Todos

### What is the Jira issue?
* [ZEPPELIN-954]

### How should this be tested?
Execute the following paragraph multiple times, and verify the table cells are still selectable.
```
%sh
echo %table
echo -e "col1\tcol2\tcol3"
echo -e "1\t2.1\tabcdefg"
```
Also try to paste anything into a cell to no avail.

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Hao Xia <hao@optimizely.com>

Closes #1059 from jasonxh/hao/hot-fix and squashes the following commits:

38d3ef4 [Hao Xia] Use the data argument consistently
1eb7fe4 [Hao Xia] Reuse the table when possible
5bd9502 [Hao Xia] Fix selection problem on second run by properly destroying hot. Also make cells readonly. Previously one were able to paste into them.

(cherry picked from commit e1b3847)
Signed-off-by: Mina Lee <minalee@apache.org>
@jasonxh jasonxh deleted the hao/hot-fix branch June 27, 2016 16:42
asfgit pushed a commit that referenced this pull request Jul 19, 2016
### What is this PR for?
Handsontable fails to display data on second run if first run did not render table due to an error in the query.
Fix for a render issue caused by #1059

### What type of PR is it?
Bug Fix

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1170

### How should this be tested?
Please ref the screenshots

### Screenshots (if appropriate)
**Before**
![old](https://cloud.githubusercontent.com/assets/2031306/16809245/35f0ced8-493d-11e6-8e1a-74c24100487a.gif)

**After**
![new](https://cloud.githubusercontent.com/assets/2031306/16809256/41a4de22-493d-11e6-9a4f-31c6ae654ceb.gif)

### Questions:
* Does the licenses files need update? n/a
* Is there breaking changes for older versions? n/a
* Does this needs documentation? n/a

Author: Renjith Kamath <renjith.kamath@gmail.com>

Closes #1182 from r-kamath/ZEPPELIN-1170 and squashes the following commits:

6f0f591 [Renjith Kamath] Merge remote-tracking branch 'upstream/master' into ZEPPELIN-1170
d63d517 [Renjith Kamath] ZEPPELIN-1170 Handsontable fails to display data on second run
asfgit pushed a commit that referenced this pull request Jul 19, 2016
Handsontable fails to display data on second run if first run did not render table due to an error in the query.
Fix for a render issue caused by #1059

Bug Fix

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

Please ref the screenshots

**Before**
![old](https://cloud.githubusercontent.com/assets/2031306/16809245/35f0ced8-493d-11e6-8e1a-74c24100487a.gif)

**After**
![new](https://cloud.githubusercontent.com/assets/2031306/16809256/41a4de22-493d-11e6-9a4f-31c6ae654ceb.gif)

* Does the licenses files need update? n/a
* Is there breaking changes for older versions? n/a
* Does this needs documentation? n/a

Author: Renjith Kamath <renjith.kamath@gmail.com>

Closes #1182 from r-kamath/ZEPPELIN-1170 and squashes the following commits:

6f0f591 [Renjith Kamath] Merge remote-tracking branch 'upstream/master' into ZEPPELIN-1170
d63d517 [Renjith Kamath] ZEPPELIN-1170 Handsontable fails to display data on second run

(cherry picked from commit 8ffd9af)
Signed-off-by: Mina Lee <minalee@apache.org>

Conflicts:
	zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
### What is this PR for?
Handsontable fails to display data on second run if first run did not render table due to an error in the query.
Fix for a render issue caused by apache#1059

### What type of PR is it?
Bug Fix

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1170

### How should this be tested?
Please ref the screenshots

### Screenshots (if appropriate)
**Before**
![old](https://cloud.githubusercontent.com/assets/2031306/16809245/35f0ced8-493d-11e6-8e1a-74c24100487a.gif)

**After**
![new](https://cloud.githubusercontent.com/assets/2031306/16809256/41a4de22-493d-11e6-9a4f-31c6ae654ceb.gif)

### Questions:
* Does the licenses files need update? n/a
* Is there breaking changes for older versions? n/a
* Does this needs documentation? n/a

Author: Renjith Kamath <renjith.kamath@gmail.com>

Closes apache#1182 from r-kamath/ZEPPELIN-1170 and squashes the following commits:

6f0f591 [Renjith Kamath] Merge remote-tracking branch 'upstream/master' into ZEPPELIN-1170
d63d517 [Renjith Kamath] ZEPPELIN-1170 Handsontable fails to display data on second run
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.

3 participants