-
Notifications
You must be signed in to change notification settings - Fork 114
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
Integrate unified base mode color and helper function #1164
Conversation
I made some changes in e-mission-common, can you update this PR?
|
…de_colors to base_modes
@JGreenlee sounds good, just made those changes in the e-mission-phone PR! |
-remove unused imports -fix missing type "BaseModeKey" --this was previously `keyof typeof BASE_MODES`. Now that BASE_MODES is in e-mission-common and is defined as a `dict`, Typescript has a harder time picking up on the types. Keeping as `string` for now.
modeColors is not in diaryHelper anymore; must be accessed from e-mission-common's `base_modes`
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1164 +/- ##
==========================================
- Coverage 30.36% 30.04% -0.33%
==========================================
Files 118 118
Lines 5256 5196 -60
Branches 1106 1162 +56
==========================================
- Hits 1596 1561 -35
+ Misses 3658 3631 -27
- Partials 2 4 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Can you also include some screenshots to show that there are no regressions before I merge?
There are no usages of this
Integrated
e-mission-common
PR which unifies the base mode colors and a helper function. Went through the code base and swapped all instances ofgetBaseModeByKey()
withbase_mode_colors.get_base_mode_by_key()
. I also convertedmodeColors
andBaseModes
indiaryHelper.ts
to now use our unified functions. IndiaryHelper.ts
I also had to add in types for one of the functions as I was getting a typescript "type unknown of distA/distB" error. I guess when I removedgetBaseModeByKey()
, the type inference couldn't figure out that they were numbers so it was throwing that error.Testing Done
I went through each component that I converted
getBaseModeByKey()
forbase_mode_colors.get_base_mode_by_key()
and tested it out to make sure it still worked properly.