You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/02/19 19:16:20 UTC
[GitHub] [superset] michael-s-molina opened a new pull request #13241: test: Tests and Storybook entry for the AsyncAceEditor component
michael-s-molina opened a new pull request #13241:
URL: https://github.com/apache/superset/pull/13241
### SUMMARY
-Adds tests for the `AsyncAceEditor` component and a storybook entry with controls.
-Moves `AsyncAceEditor` component to its own folder
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<img width="1783" alt="Screen Shot 2021-02-19 at 4 12 49 PM" src="https://user-images.githubusercontent.com/70410625/108550588-85961600-72cd-11eb-92c5-d47e418d3387.png">
### TEST PLAN
1 - Execute all tests
2 - All tests should pass
@rusackas
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud merged pull request #13241: test: add Tests and Storybook for AsyncAceEditor
Posted by GitBox <gi...@apache.org>.
ktmud merged pull request #13241:
URL: https://github.com/apache/superset/pull/13241
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #13241: test: add Tests and Storybook for AsyncAceEditor
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #13241:
URL: https://github.com/apache/superset/pull/13241#discussion_r579776846
##########
File path: superset-frontend/spec/helpers/Worker.ts
##########
@@ -21,9 +21,12 @@ class Worker {
onMessage: Function;
+ terminate: Function;
+
constructor(stringUrl: string) {
this.url = stringUrl;
this.onMessage = () => {};
+ this.terminate = () => {};
Review comment:
Could you explain why is this needed? Not sure how it this related to AsyncAceEditor.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a change in pull request #13241: test: add Tests and Storybook for AsyncAceEditor
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #13241:
URL: https://github.com/apache/superset/pull/13241#discussion_r580146037
##########
File path: superset-frontend/spec/helpers/Worker.ts
##########
@@ -21,9 +21,12 @@ class Worker {
onMessage: Function;
+ terminate: Function;
+
constructor(stringUrl: string) {
this.url = stringUrl;
this.onMessage = () => {};
+ this.terminate = () => {};
Review comment:
During `ReactAce` `willUnmount` cycle the `brace` package invokes `Worker` `terminate`.
<img width="623" alt="Screen Shot 2021-02-22 at 7 33 04 AM" src="https://user-images.githubusercontent.com/70410625/108696807-cade6180-74e0-11eb-824e-18bf54476d01.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #13241: test: add Tests and Storybook for AsyncAceEditor
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #13241:
URL: https://github.com/apache/superset/pull/13241#discussion_r580515524
##########
File path: superset-frontend/spec/helpers/Worker.ts
##########
@@ -21,9 +21,12 @@ class Worker {
onMessage: Function;
+ terminate: Function;
+
constructor(stringUrl: string) {
this.url = stringUrl;
this.onMessage = () => {};
+ this.terminate = () => {};
Review comment:
Thanks for the info! Makes sense.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #13241: test: add Tests and Storybook for AsyncAceEditor
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #13241:
URL: https://github.com/apache/superset/pull/13241#discussion_r579776846
##########
File path: superset-frontend/spec/helpers/Worker.ts
##########
@@ -21,9 +21,12 @@ class Worker {
onMessage: Function;
+ terminate: Function;
+
constructor(stringUrl: string) {
this.url = stringUrl;
this.onMessage = () => {};
+ this.terminate = () => {};
Review comment:
Do you mind noting why is this needed? Not sure how it this related to AsyncAceEditor.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org