Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add Tests and Storybook for AsyncAceEditor #13241

Merged
merged 2 commits into from Feb 22, 2021

Conversation

michael-s-molina
Copy link
Member

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

Screen Shot 2021-02-19 at 4 12 49 PM

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

Copy link
Member

@ktmud ktmud left a comment

Choose a reason for hiding this comment

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

LGTM

@ktmud ktmud changed the title test: Tests and Storybook entry for the AsyncAceEditor component test: add Tests and Storybook for AsyncAceEditor Feb 21, 2021
constructor(stringUrl: string) {
this.url = stringUrl;
this.onMessage = () => {};
this.terminate = () => {};
Copy link
Member

@ktmud ktmud Feb 21, 2021

Choose a reason for hiding this comment

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

Do you mind noting why is this needed? Not sure how it this related to AsyncAceEditor.

Copy link
Member Author

Choose a reason for hiding this comment

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

During ReactAce willUnmount cycle the brace package invokes Worker terminate.

Screen Shot 2021-02-22 at 7 33 04 AM

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the info! Makes sense.

@ktmud ktmud merged commit 741219e into apache:master Feb 22, 2021
Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

👍

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels test:component 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants