-
Notifications
You must be signed in to change notification settings - Fork 2.1k
refactor(animation): Replace enums with string literal type aliases #4287
Conversation
Refs #4225 IMO this makes the code a little easier to read/write. https://www.typescriptlang.org/docs/handbook/advanced-types.html#string-literal-types
…cript--animation-alias
Sleeping on this one - string literals might be cleaner, but the enums also provide an easy avenue to refactoring. In this case I don't think it matters as much, since the naming of |
…cript--animation-alias
I for one vastly prefer this approach... but it looks like it's failing tests in its current state. |
My only reason for using enums were for autocomplete, and avoiding spelling mistakes. I think in this case though, these can already be autocomplete'd. If you have a stronger opinion here then lets go with yours 👍 |
Codecov Report
@@ Coverage Diff @@
## feat/typescript #4287 +/- ##
===================================================
- Coverage 98.37% 98.36% -0.01%
===================================================
Files 93 93
Lines 5771 5736 -35
Branches 778 778
===================================================
- Hits 5677 5642 -35
Misses 94 94
Continue to review full report at Codecov.
|
All 621 screenshot tests passed for commit 0fd0915 vs. |
All 621 screenshot tests passed for commit 17bee8e vs. |
…cript--animation-alias
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.
one comment, but after that it looks good.
…l-components/material-components-web into feat/typescript--animation-alias
All 621 screenshot tests passed for commit 9a3a101 vs. |
All 621 screenshot tests passed for commit 439e25f vs. |
Refs #4225
IMO this makes the code a little easier to read/write.
https://www.typescriptlang.org/docs/handbook/advanced-types.html#string-literal-types