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

feat(app, shared-data): create odd boolean and choice selection screen #14775

Merged
merged 9 commits into from
Apr 2, 2024

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Apr 2, 2024

closes AUTH-123

Overview

Creates the screen for boolean and choice params on the odd

Test Plan

upload a protocol with a boolean and choice param and send it the odd. Or upload the attached protocol and change line 187 of app/src/organisms/ProtocolSetupParameters/index.tsx to return the mockData const at the top of the file.

Click on a boolean param on the setup parameters page. Observe the screen. Try to change the values and reset to default.

Now do the same thing with the choice param and make sure it works as expected.

Changelog

  • create ChooseEnum component and test
  • create the logic to update the params in the parent ProtocolSetupParameters
  • fix tests
  • rename formatRunTimeParameterValue to formatRunTimeParameterDefaultValue and create a new formatRunTimeParameterValue that returns the value updates.

Review requests

see test plan

Risk assessment

low

@jerader jerader requested a review from a team as a code owner April 2, 2024 01:31
@jerader jerader requested review from ncdiehl11 and koji and removed request for a team April 2, 2024 01:31
Comment on lines +67 to +73
<StyledText
as="h4"
textAlign={TYPOGRAPHY.textAlignLeft}
marginBottom={SPACING.spacing16}
>
{parameter.description}
</StyledText>
Copy link
Contributor

Choose a reason for hiding this comment

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

if Sue requests us to keep the space size, we may need to add style.

https://www.figma.com/file/aS5owa8kwmA0uzqzkP1BLJ?type=design&node-id=3312-62720&mode=dev#755884946

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can work on char limit in a following PR because at this moment we don't have any long case design.

@jerader jerader requested a review from koji April 2, 2024 15:57
@jerader jerader force-pushed the app_create-choose-enum-page branch from e81f386 to f6507cc Compare April 2, 2024 17:07
@jerader jerader changed the title feat(app): create odd boolean and choice selection screen feat(app, shared-data): create odd boolean and choice selection screen Apr 2, 2024
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.19%. Comparing base (f38e962) to head (43aff2f).
Report is 1 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #14775   +/-   ##
=======================================
  Coverage   67.19%   67.19%           
=======================================
  Files        2495     2495           
  Lines       71387    71387           
  Branches     8983     8983           
=======================================
  Hits        47972    47972           
  Misses      21301    21301           
  Partials     2114     2114           
Flag Coverage Δ
shared-data 75.93% <ø> (ø)

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

@jerader jerader force-pushed the app_create-choose-enum-page branch from f6507cc to 701d579 Compare April 2, 2024 17:17
@jerader jerader requested a review from ncdiehl11 April 2, 2024 17:40
buttonType="tertiaryLowLight"
buttonText={t('restore_default')}
onClickButton={() => {
setParameter(parameter.default, parameter.variableName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want to check here whether the default equals the current value. If we actually are resetting the value, we don't show the snackbar. From figma:

Reset default value is disabled when the default value is displayed and enabled once its been changed

If the user taps on the ‘Reset default value’ button when disabled, a snack bar appears informing the user the value has not been edited

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wow omg thanks so much for pointing this out!

@jerader jerader requested a review from ncdiehl11 April 2, 2024 18:13
Copy link
Collaborator

@ncdiehl11 ncdiehl11 left a comment

Choose a reason for hiding this comment

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

works well! nice.

@jerader jerader merged commit 5efb48d into edge Apr 2, 2024
44 checks passed
@jerader jerader deleted the app_create-choose-enum-page branch April 2, 2024 19:26
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