refactor(app): remove invalid type warnings for strings from atoms #11918
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## edge #11918 +/- ##
==========================================
- Coverage 74.17% 74.17% -0.01%
==========================================
Files 2164 2164
Lines 59752 59753 +1
Branches 6264 6265 +1
==========================================
- Hits 44324 44323 -1
Misses 13953 13953
- Partials 1475 1477 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jerader
approved these changes
Dec 21, 2022
app/src/atoms/Banner/index.tsx
Outdated
| @@ -119,7 +121,7 @@ export function Banner(props: BannerProps): JSX.Element { | |||
| <Flex flex="1" alignItems={ALIGN_CENTER}> | |||
| {props.children} | |||
| </Flex> | |||
| {onCloseClick && !(isCloseActionLoading ?? false) ? ( | |||
| {onCloseClick != null && !(isCloseActionLoading ?? false) ? ( | |||
There was a problem hiding this comment.
can you maybe change onCloseClick prop on line 32 to say
onCloseClick?: (() => void) | React.MouseEventHandler<HTMLButtonElement>
Also, why is the != null no longer needed?
app/src/atoms/Banner/index.tsx
Outdated
| @@ -132,7 +134,7 @@ export function Banner(props: BannerProps): JSX.Element { | |||
| )} | |||
| </Btn> | |||
| ) : null} | |||
| {isCloseActionLoading && ( | |||
| {(isCloseActionLoading ?? false) && ( | |||
There was a problem hiding this comment.
why do we not need the ?? false here?
There was a problem hiding this comment.
isCloseActionLoading?: boolean
maybe add false as the default value and keep the {isCloseActionLoading && (
@jerader thank you for reviewing. I will address your feedback. |
|
y3rsh
added a commit
that referenced
this pull request
Jan 6, 2023
* origin/edge: (34 commits) refactor(app): update desktop robot settings calibration section for OT-3 (#11942) feat(hardware): add CAN message to update motor position from encoders (#11868) ci(monorepo): upgrade windows versions on github workflows (#11940) feat(app): implement useCalibrationTaskList hook (#11894) feat(app, app-shell, app-shell-odd): create node layer for ODD (#11944) refactor(app): Remove recalibrate option from POC and TLC overflow menus [RAUT-93] (#11915) fix(api): home z should home gripper z too (#11950) refactor(shared-data): gripper use force polynomial function (#11946) refactor(api): move `ModuleGeometry` to legacy protocol core module (#11939) refactor(api): deprecate `ModuleContext.geometry` (#11938) chore(usb-bridge): add usb-bridge tests to test-py, add restart to push-ot3 (#11937) Revert "feat(app-shell-odd): create node layer for ODD (#11852)" (#11941) feat(app-shell-odd): create node layer for ODD (#11852) feature(hardware): add a warning style to can_mon and an "estop_released" error id (#11924) fix(hardware): Remove while loop and rely on number_of_messages when parsing motor position response. (#11929) fix(hardware): save can_comm / can_mon logs to read-write location (#11933) feat(api): Support 96 channel in the hardware controller (#11866) refactor(app): revert run a protocol from devices pages (#11909) refactor(app): remove warnings (#11922) refactor(app): remove invalid type warnings for strings from atoms (#11918) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Add String() to remove warnings from atoms
The lines I added
String()should bestringsince most of them are CSS property values for test cases(not null / not undefined).This is part of RAUT-74
The number of warnings
Changelog
Review requests
Risk assessment
low