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.

Sign up for GitHub

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

Jump to bottom

feat(app): add Fixture table to Modules & deck screen under protocol setup #13757

Merged
merged 7 commits into from
Oct 12, 2023
Merged

feat(app): add Fixture table to Modules & deck screen under protocol setup #13757

merged 7 commits into from
Oct 12, 2023

Conversation

koji
Copy link
Contributor

@koji koji commented Oct 10, 2023 •
edited
Loading

Overview

Add Fixture table without status. The status will be implemented by RAUT-670

Screenshot 2023-10-11 at 3 03 30 PM

close RAUT-668

Test Plan

  1. push this branch to a dev kit or Flex
  2. Set up a protocol
  3. Tap Modules & deck
    Check the Fixture table is displayed under the Modules table

Changelog

  • create FixtureTable component and add the very basic test
  • update the text in ProtocolSetup screen Modules -> Modules & deck
  • update tests for the above changes

Review requests

Risk assessment

low

@codecov
Copy link

codecov bot commented Oct 10, 2023 •
edited
Loading

Codecov Report

Merging #13757 (959279b) into edge (f73772c) will decrease coverage by 0.05%.
Report is 5 commits behind head on edge.
The diff coverage is n/a.

❗ Current head 959279b differs from pull request most recent head aa5f07c. Consider uploading reports for the commit aa5f07c to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #13757      +/-   ##
==========================================
- Coverage   70.95%   70.90%   -0.05%     
==========================================
  Files        2453     1604     -849     
  Lines       69022    53338   -15684     
  Branches     8257     3646    -4611     
==========================================
- Hits        48976    37821   -11155     
+ Misses      18094    14929    -3165     
+ Partials     1952      588    -1364
Flag Coverage Δ
app 44.12% <ø> (-24.73%) ⬇️

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

see 849 files with indirect coverage changes

@koji koji requested review from brenthagen, jerader and a team October 11, 2023 19:04
@koji koji marked this pull request as ready for review October 11, 2023 19:05
@koji koji requested a review from a team as a code owner October 11, 2023 19:05
@koji
Copy link
Contributor Author

koji commented Oct 11, 2023 •
edited
Loading

  • update tests

@koji koji requested a review from a team as a code owner October 11, 2023 21:51
koji and others added 2 commits October 12, 2023 10:17
@koji koji requested review from brenthagen and a team and removed request for a team October 12, 2023 15:12
brenthagen
brenthagen approved these changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@brenthagen brenthagen left a comment

Choose a reason for hiding this comment

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

LGTM ! once tests pass

app/src/organisms/ProtocolSetupModules/index.tsx
onClick={() => setSetupScreen('prepare to run')}
<ChildNavigation
header={enableDeckConfig ? t('modules_and_deck') : t('modules')}
onClickBack={() => setSetupScreen('prepare to run')}
/>
<SmallButton
Copy link
Contributor

Choose a reason for hiding this comment

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

this "Setup Instructions" button needs to be passed to ChildNavigation - looks like ChildNavigation props need to be modified to accept more button props https://www.figma.com/file/OIdG64Q5cgvEw82ish5Eee/Design-System-(Flex)?type=design&node-id=1224-66324&mode=design&t=mqw4k8GQxVwbDH79-4

this could be done now or in another PR

Copy link
Contributor

Choose a reason for hiding this comment

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

currently "Setup Instructions" not visible:

Screen Shot 2023-10-12 at 11 19 14 AM

Copy link
Contributor Author

@koji koji Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I will work on ChildNavigation in another PR.
https://opentrons.atlassian.net/browse/RAUT-794

@koji koji merged commit 6dced1a into edge Oct 12, 2023
21 of 22 checks passed
@koji koji deleted the feat_add-fixture-table-to-ODD branch April 17, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brenthagen brenthagen brenthagen approved these changes

@jerader jerader Awaiting requested review from jerader

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@koji @brenthagen