Skip to content
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

chore(app): extend props of intervention modal #17533

Merged

Conversation

TamarZanzouri
Copy link
Contributor

@TamarZanzouri TamarZanzouri commented Feb 14, 2025

Overview

closes https://opentrons.atlassian.net/browse/EXEC-1224.
Add subtext and tagtext props to intervention modal.

Test Plan and Hands on Testing

Can follow storybook for changes. (not passing these props yet so can only view in storybook).
https://s3-us-west-2.amazonaws.com/opentrons-components/EXEC-1224-component-update-intervention-info-component/index.html?path=/docs/app-molecules-interventionmodal-interventioncontent-interventioninfo--docs

Changelog

Added 2 new props to intervention modal. subtext and tagtext.
Added conditional display.
fixed bug in storybook where it didnt show the location label.

Review requests

changes make sense? do we want to fix storybook rendering the slots?

Risk assessment

low. added 2 optional props.

@TamarZanzouri TamarZanzouri requested a review from a team as a code owner February 14, 2025 17:20
@TamarZanzouri TamarZanzouri requested review from ncdiehl11, sfoster1 and smb2268 and removed request for a team and ncdiehl11 February 14, 2025 17:20
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good once we have that default text internationalized

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple more small things

Copy link

codecov bot commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 0% with 24 lines in your changes missing coverage. Please review.

Project coverage is 27.17%. Comparing base (f5cdb6b) to head (2de0607).
Report is 37 commits behind head on edge.

Files with missing lines Patch % Lines
...tionModal/InterventionContent/InterventionInfo.tsx 0.00% 23 Missing ⚠️
...rrorRecoveryFlows/shared/LeftColumnLabwareInfo.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17533      +/-   ##
==========================================
+ Coverage   26.17%   27.17%   +1.00%     
==========================================
  Files        2835     2840       +5     
  Lines      217682   218818    +1136     
  Branches     9276     9837     +561     
==========================================
+ Hits        56971    59474    +2503     
+ Misses     160694   159323    -1371     
- Partials       17       21       +4     
Flag Coverage Δ
app 3.42% <0.00%> (+0.01%) ⬆️
components 4.66% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rrorRecoveryFlows/shared/LeftColumnLabwareInfo.tsx 0.00% <0.00%> (ø)
...tionModal/InterventionContent/InterventionInfo.tsx 0.00% <0.00%> (ø)

... and 120 files with indirect coverage changes

text={t('labware_quantity', { quantity: props.tagtext })}
shrinkToContent={true}
/>
) : null}
</Flex>
<Divider
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the designs we should now only display this divider if a tag is present 🏷️

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Copy link
Contributor

@smb2268 smb2268 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look great! If Mel says everything looks good we should merge 😃

@TamarZanzouri
Copy link
Contributor Author

Mel approved :-)

@TamarZanzouri TamarZanzouri merged commit 992a6b9 into edge Feb 21, 2025
40 checks passed
@TamarZanzouri TamarZanzouri deleted the EXEC-1224-component-update-intervention-info-component branch February 21, 2025 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants