Skip to content

ZEPPELIN-1170 Handsontable fails to display data on second run#1182

Closed
r-kamath wants to merge 2 commits into
apache:masterfrom
r-kamath:ZEPPELIN-1170
Closed

ZEPPELIN-1170 Handsontable fails to display data on second run#1182
r-kamath wants to merge 2 commits into
apache:masterfrom
r-kamath:ZEPPELIN-1170

Conversation

@r-kamath
Copy link
Copy Markdown
Member

@r-kamath r-kamath commented Jul 13, 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

After
new

Questions:

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

@Leemoonsoo
Copy link
Copy Markdown
Member

@r-kamath Thanks for the fix. How this fix is different from #1171 ?

@prabhjyotsingh
Copy link
Copy Markdown
Contributor

LGTM!

This has been happening in my machine as well for quite some time. Thanks @r-kamath for the fix.

@corneadoug, @Leemoonsoo Please review will merge this a hot fix.

@r-kamath
Copy link
Copy Markdown
Member Author

@Leemoonsoo My bad, I missed your PR.
Difference in #1171 and this pr

  • $scope.hot = null; is not necessary
  • updateSettings to update resultRows is one more call and is not doing anything significant
    Otherwise the change is pretty much same.

@r-kamath
Copy link
Copy Markdown
Member Author

@Leemoonsoo if #1171 is ready to merge. Please feel free to discard this PR and mark the jira issue as duplicate. Thanks :)

@Leemoonsoo
Copy link
Copy Markdown
Member

I think we can merge #1171 and then merge #1182, while this PR includes different improvements. ($scope.hot = null and updateSettings)

I didn't marked #1171 as a hotfix, but let me merge #1171 quickly, and then let's merge #1182.

@r-kamath r-kamath changed the title [HOTFIX] ZEPPELIN-1170 Handsontable fails to display data on second run ZEPPELIN-1170 Handsontable fails to display data on second run Jul 13, 2016
@prabhjyotsingh
Copy link
Copy Markdown
Contributor

@Leemoonsoo Yes, I agree, we should merge #1171.

@bzz
Copy link
Copy Markdown
Member

bzz commented Jul 14, 2016

So what is the consensus, shall we actually rebase this one on master and merge, or just close, as soon as #1171 is merged?

@corneadoug
Copy link
Copy Markdown
Contributor

#1171 is merged already, and this one has been rebased

@bzz
Copy link
Copy Markdown
Member

bzz commented Jul 14, 2016

@corneadoug from what you say, it's still not clear what's the plan with this one.

I do not think it was rebased, where rebased = git rebase - that way all the commits would show up AFTER the conversation as history gets re-written.

Also we would not have commit messages like "Merge remote-tracking branch 'upstream/master' into ZEPPELIN-1170" in case of rebase.

@corneadoug
Copy link
Copy Markdown
Contributor

Tested, LGTM

@corneadoug
Copy link
Copy Markdown
Contributor

Merging if there is no more discussions

@asfgit asfgit closed this in 8ffd9af Jul 19, 2016
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.

5 participants