[ZEPPELIN-1722] Modernize visualization/transformation using ES6 class and module syntax #1815
Conversation
|
\cc @1ambda @felizbear |
| } | ||
| }; | ||
|
|
||
| _renderSetting(targetEl, template, scope) { |
There was a problem hiding this comment.
can we make it a function instead of method? there is no reason for this to be method to my understanding
There was a problem hiding this comment.
Can class have function? or you mean function inside of method?
There was a problem hiding this comment.
no, i mean outside of the class definition. don't forget, it's not java. we're talking about a module called visualization here. this module exports a class, but it can have private code as well, which could be, for example, a function renderSetting. does it make sense?
There was a problem hiding this comment.
ah i see make sense. let me update it.
| if (template.split('\n').length === 1 && | ||
| template.endsWith('.html')) { // template is url | ||
| var self = this; | ||
| this._templateRequest(template).then(function(t) { |
There was a problem hiding this comment.
instead of confusing self = this it is better to use =>, e.g. templateRequest().then(t => {/* use this here safely */}); it will bind the context of the outer scope
There was a problem hiding this comment.
Removing self and use => is not part of this PR's scope. but good to know. let me update this, too.
|
I generally wish that api would be designed more like d3 or jquery, e.g. something along the lines of |
|
Addressed comments. I don't think style |
|
Merge to master if there're no more discussions |
### What is this PR for? After #1815 was merged, ``` uncaught TypeError: this._isNumeric is not a function at ColumnSettings._numericValidator [as validator] (handsonHelper.js:172) at handsontable.js:5181 ``` is shown when click "Numeric" in the result table like below. <img src="https://cloud.githubusercontent.com/assets/10060731/21749365/d1d700ee-d5e0-11e6-9f25-65ebb3ea313a.gif" width="450px"> Since ES6 no longer supports autobind for `this`, seems it needs to be bound in the constructor. ### What type of PR is it? Bug Fix ### What is the Jira issue? [ZEPPELIN-1924](https://issues.apache.org/jira/browse/ZEPPELIN-1924) ### How should this be tested? **To reproduce** In master, go to Spark tutorial note and click <img width="230" alt="screen shot 2017-01-08 at 8 32 05 pm" src="https://cloud.githubusercontent.com/assets/10060731/21749412/8a123318-d5e1-11e6-9a65-a84e443c385c.png"> `Numeric`. Then the error msg will be shown in browser dev console. With this patch, this error msg won't be shown up anymore :) ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: AhyoungRyu <fbdkdud93@hanmail.net> Closes #1870 from AhyoungRyu/ZEPPELIN-1924 and squashes the following commits: 7d5bf9e [AhyoungRyu] Bind _numericValidator in the constructor
2 participants
What is this PR for?
Modernize visualization/transformation using ES6 class and module syntax.
And remove global variable 'zeppelin'
What type of PR is it?
Refactoring
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1722
Questions: