-
Notifications
You must be signed in to change notification settings - Fork 14k
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
refactor: migrate DeleteComponentButton to TypeScript #18136
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18136 +/- ##
==========================================
+ Coverage 65.95% 66.29% +0.34%
==========================================
Files 1584 1595 +11
Lines 62046 62590 +544
Branches 6273 6302 +29
==========================================
+ Hits 40920 41494 +574
+ Misses 19505 19451 -54
- Partials 1621 1645 +24
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
superset-frontend/src/dashboard/components/DeleteComponentButton.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/DeleteComponentButton.tsx
Outdated
Show resolved
Hide resolved
@ad-m I've made mentioned changes. |
superset-frontend/src/dashboard/components/DeleteComponentButton.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good. Would you be interested in moving it to a functional component as well?
@geido I've moved it to functional component |
Thanks @Scrip7! Now running CI |
@geido CI is done! |
/testenv up |
@geido Ephemeral environment spinning up at http://34.212.22.182:8080. Credentials are |
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
Migrated DeleteComponentButton to TypeScript to apply direction outlined in #18100 .
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No changes
TESTING INSTRUCTIONS
The storybook can be used to verify those changes.
ADDITIONAL INFORMATION
@ad-m , @zhaoyongjie , @kristw