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

refactor(components): Refactor select components for LPC Redesign #17622

Merged
merged 8 commits into from
Mar 5, 2025

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Feb 28, 2025

Closes EXEC-1242 and EXEC-1260

Overview

This PR updates a couple components as required for the LPC Redesign. There is a lot of noise in this PR. The first two commits are the non-lint solving ones.

RadioButton

See here. Just adds better defined subtext, supporting vertical and horizontal placements. This requires updating a couple implementations, too.

ListItem

See here. Just adds some new states and background colors. Renames noActive to default per Design, too.

Test Plan and Hands on Testing

Risk assessment

low

@mjhuff mjhuff requested review from koji, sfoster1, ncdiehl11, jerader and a team February 28, 2025 22:37
@mjhuff mjhuff requested a review from a team as a code owner February 28, 2025 22:37
@mjhuff mjhuff force-pushed the components_refactor-select-components-for-lpc-redesign branch from ac284fb to d2b4460 Compare February 28, 2025 22:39
@mjhuff mjhuff requested a review from a team as a code owner February 28, 2025 22:39
@mjhuff mjhuff force-pushed the components_refactor-select-components-for-lpc-redesign branch from d2b4460 to 2061cd7 Compare February 28, 2025 22:46
@@ -11,10 +11,14 @@ export * from './ListItemChildren'

export type ListItemType =
| 'error'
| 'noActive'
| 'default'
Copy link
Contributor

@koji koji Feb 28, 2025

Choose a reason for hiding this comment

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

🥇
thank you for doing this actually Nick and I talked about updating this.
protocol-designer uses this a lot. If you want us(auth team) to update them, let us know.

Copy link
Contributor Author

@mjhuff mjhuff Mar 1, 2025

Choose a reason for hiding this comment

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

Sounds good! I went ahead and did the changes since they were failing lint (below commit). It appears there's one other component that still uses noActive, which is ListButton.

Copy link

codecov bot commented Mar 1, 2025

Codecov Report

Attention: Patch coverage is 88.78505% with 12 lines in your changes missing coverage. Please review.

Project coverage is 64.48%. Comparing base (507da17) to head (341820c).
Report is 8 commits behind head on edge.

Files with missing lines Patch % Lines
.../pages/Designer/ProtocolSteps/Timeline/Substep.tsx 0.00% 3 Missing ⚠️
...r/src/pages/Designer/ProtocolSteps/StepSummary.tsx 0.00% 2 Missing ⚠️
...s/ODD/NetworkSettings/SelectAuthenticationType.tsx 75.00% 1 Missing ⚠️
...low/QuickTransferAdvancedSettings/BaseSettings.tsx 0.00% 1 Missing ⚠️
...src/organisms/AssignLiquidsModal/LiquidToolbox.tsx 0.00% 1 Missing ⚠️
...organisms/EditInstrumentsModal/PipetteOverview.tsx 0.00% 1 Missing ⚠️
...rc/pages/CreateNewProtocolWizard/SelectModules.tsx 0.00% 1 Missing ⚠️
...tocolSteps/StepForm/PipetteFields/TiprackField.tsx 0.00% 1 Missing ⚠️
...StepTools/AbsorbanceReaderTools/Initialization.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             edge   #17622       +/-   ##
===========================================
+ Coverage   25.68%   64.48%   +38.80%     
===========================================
  Files        2851     2820       -31     
  Lines      219457   217593     -1864     
  Branches    17971    18466      +495     
===========================================
+ Hits        56363   140321    +83958     
+ Misses     163079    77084    -85995     
- Partials       15      188      +173     
Flag Coverage Δ
protocol-designer 18.96% <58.87%> (+0.02%) ⬆️
step-generation 4.37% <0.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
app/src/molecules/HeadlineTagBtn/index.tsx 100.00% <100.00%> (+100.00%) ⬆️
...ompatibleModule/IncompatibleModuleODDModalBody.tsx 100.00% <100.00%> (+100.00%) ⬆️
...p/src/organisms/ODD/QuickTransferFlow/Overview.tsx 93.54% <100.00%> (+93.54%) ⬆️
...ansferFlow/QuickTransferAdvancedSettings/index.tsx 92.66% <100.00%> (+92.66%) ⬆️
...anisms/ODD/QuickTransferFlow/SelectDestLabware.tsx 94.11% <100.00%> (+94.11%) ⬆️
.../organisms/ODD/QuickTransferFlow/SelectPipette.tsx 95.12% <100.00%> (+95.12%) ⬆️
...isms/ODD/QuickTransferFlow/TipManagement/index.tsx 92.77% <100.00%> (+92.77%) ⬆️
components/src/atoms/ListItem/index.tsx 100.00% <100.00%> (ø)
components/src/atoms/buttons/RadioButton.tsx 100.00% <100.00%> (+9.75%) ⬆️
components/src/atoms/index.ts 100.00% <100.00%> (ø)
... and 24 more

... and 1610 files with indirect coverage changes

@mjhuff mjhuff force-pushed the components_refactor-select-components-for-lpc-redesign branch from 99b90ac to 2a74fea Compare March 3, 2025 19:24
@mjhuff mjhuff force-pushed the components_refactor-select-components-for-lpc-redesign branch from 3f02bc9 to 3e6c15a Compare March 3, 2025 19:32
@smb2268 smb2268 self-requested a review March 5, 2025 14:02
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 & storybook look good to me!

Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

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

did a smoke test for the PD and there were no issues
thank you for updating type!

@mjhuff mjhuff merged commit f5acc2b into edge Mar 5, 2025
63 checks passed
@mjhuff mjhuff deleted the components_refactor-select-components-for-lpc-redesign branch March 5, 2025 17:16
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