feat(app): extend SectionList component for Generic Step Screen #8513
Conversation
Codecov Report
@@ Coverage Diff @@
## edge #8513 +/- ##
==========================================
+ Coverage 74.29% 74.31% +0.01%
==========================================
Files 1693 1693
Lines 45557 45568 +11
Branches 4634 4639 +5
==========================================
+ Hits 33848 33865 +17
+ Misses 10907 10901 -6
Partials 802 802
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jerader
requested review from
Kadee80,
shlokamin and
a team
and removed request for
a team
jerader
changed the title
| import type { LabwarePositionCheckStep } from './types' | ||
| import { useIntroInfo } from './hooks' |
There was a problem hiding this comment.
nit: the type import should go last
| export const GenericStepScreen = ( | ||
| export function GenericStepScreen( |
There was a problem hiding this comment.
how come this got changed from a function expression to a function declaration?
There was a problem hiding this comment.
oh this is outdated now, ignore! 😄
| completedSections != null && completedSections.includes(section) | ||
| let iconBackgroundColor = C_DISABLED | ||
| if (section === currentSection) { | ||
| iconBackgroundColor = '#00c3e6' |
There was a problem hiding this comment.
can we use C_SELECTED_DARK here instead of the string literal?
| backgroundColor={C_NEAR_WHITE} | ||
| > | ||
| {sections.map((section, index) => { | ||
| const sectionTextOrBackgroundColor = |
There was a problem hiding this comment.
can we rename sectionTextOrBackgroundColor to sectionTextColor?
while it's being used to inform the Box element's backgroundColor, it's really being used by the Text element as it's color attribute
| : C_DARK_GRAY | ||
| const isCompleted = | ||
| completedSections != null && completedSections.includes(section) | ||
| let iconBackgroundColor = C_DISABLED |
There was a problem hiding this comment.
can we rename iconBackgroundColor to backgroundColor? this variable is being used whether or not we render an icon
| }) | ||
| it('renders LabwarePositionCheckStepDetail component', () => { | ||
| const { getByText } = render(props) | ||
| expect(getByText('Mock Labware Position Check Step Detail')).toBeTruthy() | ||
| }) | ||
| it('renders GenericStepScreenNav component', () => { | ||
| const { getByText } = render(props) | ||
| expect(getByText('Mock SectionList')).toBeTruthy() |
There was a problem hiding this comment.
this can be replaced with just getByText('Mock SectionList')
| const PICKUP_TIP_LABWARE_ID = 'PICKUP_TIP_LABWARE_ID' | ||
| const PRIMARY_PIPETTE_ID = 'PRIMARY_PIPETTE_ID' | ||
| const mockLabwarePositionCheckStepTipRack = { | ||
| const MOCK_SECTION = ['PRIMARY_PIPETTE_TIPRACKS' as Section] |
There was a problem hiding this comment.
since this variable represents multiple sections, can we call it MOCK_SECTIONS (plural)?
| }) | ||
| it('renders a 2 step Nav with 1 pipette', () => { | ||
| const { getByText } = render(props) | ||
| expect(getByText('1')).toHaveStyle('backgroundColor: #00c3e6') |
There was a problem hiding this comment.
can we use the C_SELECTED_DARK variable here? that way if we ever change color palettes we wont have to update all of our tests again 😄
Overview
closes #8500
Changelog
SectionListand create testPositionCheckNavtoSectionListReview requests
SectionListto make sure it has all the AC in the ticketRisk assessment
low, behind ff