-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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 UndoRedoKeyListeners to typescript #30654
refactor: Migrate UndoRedoKeyListeners to typescript #30654
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #30654 +/- ##
===========================================
+ Coverage 60.48% 70.80% +10.31%
===========================================
Files 1931 1984 +53
Lines 76236 80042 +3806
Branches 8568 9156 +588
===========================================
+ Hits 46114 56675 +10561
+ Misses 28017 21133 -6884
- Partials 2105 2234 +129
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
9d18a7e
to
d5dfcad
Compare
/testenv up |
@geido Ephemeral environment spinning up at http://35.88.171.186:8080. Credentials are |
const propTypes = { | ||
onUndo: PropTypes.func.isRequired, | ||
onRedo: PropTypes.func.isRequired, | ||
type UndoRedoKeyListenersProps = { |
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.
nit: there's a ts type helper Pick
designed for situations just like this one. We could use it here like this:
type UndoRedoKeyListenersProps = Pick<HeaderProps, 'onUndo' | 'onRedo'>;
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.
But your code is fine as well, so let me know if you'd rather keep it as it is and I'll merge the PR
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.
nit: there's a ts type helper
Pick
designed for situations just like this one. We could use it here like this:type UndoRedoKeyListenersProps = Pick<HeaderProps, 'onUndo' | 'onRedo'>;
I preferred to create a specific type, thank you.
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.
But your code is fine as well, so let me know if you'd rather keep it as it is and I'll merge the PR
If you’d like, I can modify it.
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.
All good, merged 🙂
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
Migrate UndoRedoKeyListeners to typescript
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N.A
TESTING INSTRUCTIONS
All tests should pass
ADDITIONAL INFORMATION