Skip to content

Commit

Permalink
feat(app): Implement two cols RadioButton controller (#15634)
Browse files Browse the repository at this point in the history
<!--
Thanks for taking the time to open a pull request! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview

<!--
Use this section to describe your pull-request at a high level. If the
PR addresses any open issues, please tag the issues here.
-->

AUTH-521: Implementing functionality to control radio buttons in two
columns for selecting a CSV file on robot or on USB. When users tabbed
back button after they selected a file, updating parameters with file id
and file name for a file selected on robot or with file path and file
name for a file selected on USB.

AUTH-558: Replacing the confirm selection button with an inline
notification and updating back button control.

# Test Plan

<!--
Use this section to describe the steps that you took to test your Pull
Request.
If you did not perform any testing provide justification why.

OT-3 Developers: You should default to testing on actual physical
hardware.
Once again, if you did not perform testing against hardware, justify
why.

Note: It can be helpful to write a test plan before doing development

Example Test Plan (HTTP API Change)

- Verified that new optional argument `dance-party` causes the robot to
flash its lights, move the pipettes,
then home.
- Verified that when you omit the `dance-party` option the robot homes
normally
- Added protocol that uses `dance-party` argument to G-Code Testing
Suite
- Ran protocol that did not use `dance-party` argument and everything
was successful
- Added unit tests to validate that changes to pydantic model are
correct

-->

- Clicked/tabbed on the radio buttons with file name to see if it's
being selected (changing background color)
- Printed out the file name in the console to check if the csv file
information (id/filePath and fileName) being correctly pass back to the
parameters when tabbed back button on the Choose CSV File screen.
- Redirected to Parameters screen when tabbed on the back button 

<img width="839" alt="Screenshot 2024-07-15 at 9 56 42 AM"
src="https://github.com/user-attachments/assets/2c6ffd93-78d8-4eef-830c-3c04ebc29439">


# Changelog

<!--
List out the changes to the code in this PR. Please try your best to
categorize your changes and describe what has changed and why.

Example changelog:
- Fixed app crash when trying to calibrate an illegal pipette
- Added state to API to track pipette usage
- Updated API docs to mention only two pipettes are supported

IMPORTANT: MAKE SURE ANY BREAKING CHANGES ARE PROPERLY COMMUNICATED
-->

- Added a state with type CsvFileFileType to store csv file information
- Replaced the small button element with an inline notification element
inside child navigation
- Updated the parameter's value type to { boolean | string | number |
CsvFileFileType } where CsvFileFileType contains id, file, file path,
and file name
- Removed csvFileInfo and setcsvFileInfo state in both chooseCsvFile and
protocolSetupParameters/index files
- Updated tests

# Review requests

<!--
Describe any requests for your reviewers here.
-->

# Risk assessment

<!--
Carefully go over your pull request and look at the other parts of the
codebase it may affect. Look for the possibility, even if you think it's
small, that your change may affect some other part of the system - for
instance, changing return tip behavior in protocol may also change the
behavior of labware calibration.

Identify the other parts of the system your codebase may affect, so that
in addition to your own review and testing, other people who may not
have the system internalized as much as you can focus their attention
and testing there.
-->

---------

Co-authored-by: shiyaochen <[email protected]>
Co-authored-by: Max Marrone <[email protected]>
Co-authored-by: Caila Marashaj <[email protected]>
Co-authored-by: Ryan Howard <[email protected]>
Co-authored-by: Seth Foster <[email protected]>
Co-authored-by: aaron-kulkarni <[email protected]>
Co-authored-by: Shlok Amin <[email protected]>
Co-authored-by: koji <[email protected]>
Co-authored-by: pmoegenburg <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: y3rsh <[email protected]>
Co-authored-by: Josh McVey <[email protected]>
Co-authored-by: Brayan Almonte <[email protected]>
Co-authored-by: Derek Maggio <[email protected]>
Co-authored-by: Nicholas Shiland <[email protected]>
Co-authored-by: Brent Hagen <[email protected]>
Co-authored-by: shiyaochen <[email protected]>
  • Loading branch information
18 people authored Jul 15, 2024
1 parent ec2aa91 commit f2ab3d3
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 53 deletions.
1 change: 1 addition & 0 deletions app/src/assets/localization/en/protocol_setup.json
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@
"update_deck": "Update deck",
"updated": "Updated",
"usb_connected_no_port_info": "USB Port Connected",
"usb_drive_notification": "Leave USB drive attached until run starts",
"usb_port_connected": "USB Port {{port}}",
"usb_port_number": "USB-{{port}}",
"value_out_of_range_generic": "Value must be in range",
Expand Down
86 changes: 49 additions & 37 deletions app/src/organisms/ProtocolSetupParameters/ChooseCsvFile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react'
import { useTranslation } from 'react-i18next'
import { useSelector } from 'react-redux'
import { css } from 'styled-components'
import isEqual from 'lodash/isEqual'
import last from 'lodash/last'

import {
Expand All @@ -20,49 +21,52 @@ import { getShellUpdateDataFiles } from '../../redux/shell'
import { ChildNavigation } from '../ChildNavigation'
import { EmptyFile } from './EmptyFile'

import type { CsvFileParameter } from '@opentrons/shared-data'
import type { CsvFileParameter, CsvFileFileType } from '@opentrons/shared-data'
import type { CsvFileData } from '@opentrons/api-client'

interface ChooseCsvFileProps {
protocolId: string
handleGoBack: () => void
// ToDo (kk:06/18/2024) null will be removed when implemented required part
parameter: CsvFileParameter | null
setParameter: (value: boolean | string | number, variableName: string) => void
csvFileInfo: string
setCsvFileInfo: (fileInfo: string) => void
parameter: CsvFileParameter
setParameter: (
value: boolean | string | number | CsvFileFileType,
variableName: string
) => void
}

export function ChooseCsvFile({
protocolId,
handleGoBack,
parameter,
setParameter,
csvFileInfo,
setCsvFileInfo,
}: ChooseCsvFileProps): JSX.Element {
const { t } = useTranslation('protocol_setup')

const csvFilesOnUSB = useSelector(getShellUpdateDataFiles) ?? []

const csvFilesOnRobot = useAllCsvFilesQuery(protocolId).data?.data.files ?? []

// ToDo (06/20/2024) this will removed when working on AUTH-521
// const handleOnChange = (newValue: string | number | boolean): void => {
// setParameter(newValue, parameter?.variableName ?? 'csvFileId')
// }
const initialFileObject: CsvFileFileType = parameter.file ?? {}
const [csvFileSelected, setCsvFileSelected] = React.useState<CsvFileFileType>(
initialFileObject
)

const handleConfirmSelection = (): void => {
// ToDo (kk:06/18/2024) wire up later
const handleBackButton = (): void => {
if (!isEqual(csvFileSelected, initialFileObject)) {
setParameter(csvFileSelected, parameter.variableName)
}
handleGoBack()
}

return (
<>
<ChildNavigation
header={t('choose_csv_file')}
onClickBack={handleGoBack}
buttonType="primary"
buttonText={t('confirm_selection')}
onClickButton={handleConfirmSelection}
onClickBack={handleBackButton}
inlineNotification={{
type: 'neutral',
heading: t('usb_drive_notification'),
}}
/>
<Flex
marginTop="7.75rem"
Expand All @@ -84,8 +88,11 @@ export function ChooseCsvFile({
key={csv.id}
data-testid={csv.id}
buttonLabel={csv.name}
buttonValue={csv.id}
onChange={() => {}}
buttonValue={`${csv.id}`}
onChange={() => {
setCsvFileSelected({ id: csv.id, fileName: csv.name })
}}
isSelected={csvFileSelected?.id === csv.id}
/>
))
) : (
Expand All @@ -99,23 +106,28 @@ export function ChooseCsvFile({
</LegacyStyledText>
<Flex css={LIST_CONTAINER_STYLE}>
{csvFilesOnUSB.length !== 0 ? (
csvFilesOnUSB.map(csv => (
<>
{csv.length !== 0 && last(csv.split('/')) !== undefined ? (
<RadioButton
key={last(csv.split('/'))}
data-testid={`${last(csv.split('/'))}`}
buttonLabel={last(csv.split('/')) ?? 'default'}
buttonValue={csv}
onChange={() => {
// ToDO this will be implemented AUTH-521
// handleOnChange(option.value)
setCsvFileInfo(csv)
}}
/>
) : null}
</>
))
csvFilesOnUSB.map(csvFilePath => {
const fileName = last(csvFilePath.split('/'))
return (
<>
{csvFilePath.length !== 0 && fileName !== undefined ? (
<RadioButton
key={fileName}
data-testid={fileName}
buttonLabel={fileName ?? 'default'}
buttonValue={csvFilePath}
onChange={() => {
setCsvFileSelected({
filePath: csvFilePath,
fileName: fileName,
})
}}
isSelected={csvFileSelected?.filePath === csvFilePath}
/>
) : null}
</>
)
})
) : (
<EmptyFile />
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ vi.mock('../EmptyFile')
const mockHandleGoBack = vi.fn()
const mockSetParameter = vi.fn()
const mockParameter: CsvFileParameter = {} as any
const mockSetFileInfo = vi.fn()
const PROTOCOL_ID = 'fake_protocol_id'
const mockUsbData = [
'/media/mock-usb-drive/mock-file1.csv',
Expand Down Expand Up @@ -64,8 +63,6 @@ describe('ChooseCsvFile', () => {
handleGoBack: mockHandleGoBack,
parameter: mockParameter,
setParameter: mockSetParameter,
csvFileInfo: 'mockFileId',
setCsvFileInfo: mockSetFileInfo,
}
vi.mocked(getLocalRobot).mockReturnValue(mockConnectedRobot)
vi.mocked(EmptyFile).mockReturnValue(<div>mock EmptyFile</div>)
Expand All @@ -80,7 +77,7 @@ describe('ChooseCsvFile', () => {
screen.getByText('Choose CSV file')
screen.getByText('CSV files on robot')
screen.getByText('CSV files on USB')
screen.getByText('Confirm selection')
screen.getByText('Leave USB drive attached until run starts')
})

it('should render csv file names', () => {
Expand All @@ -95,13 +92,71 @@ describe('ChooseCsvFile', () => {
screen.getByText('mock-file3.csv')
})

it('should call a mock function when tapping back button', () => {
it('should call a mock function when tapping back button + without selecting a csv file', () => {
render(props)

fireEvent.click(screen.getAllByRole('button')[0])
expect(props.setParameter).not.toHaveBeenCalled()
expect(mockHandleGoBack).toHaveBeenCalled()
})

it('should render a selected radio button in Robot side when tapped', () => {
when(useAllCsvFilesQuery)
.calledWith(PROTOCOL_ID)
.thenReturn(mockDataOnRobot as any)
render(props)

const selectedCsvFileOnRobot = screen.getByLabelText('rtp_mock_file2.csv')
fireEvent.click(selectedCsvFileOnRobot)
expect(selectedCsvFileOnRobot).toBeChecked()
})

it('should render a selected radio button in USB side when tapped', () => {
render(props)

const selectCsvFileOnUsb = screen.getByLabelText('mock-file2.csv')
fireEvent.click(selectCsvFileOnUsb)
expect(selectCsvFileOnUsb).toBeChecked()
})

it('call mock function (setParameter) with fileId + fileName when the selected file is a csv on Robot + tapping back button', () => {
when(useAllCsvFilesQuery)
.calledWith(PROTOCOL_ID)
.thenReturn(mockDataOnRobot as any)
render(props)

const csvFileOnRobot = screen.getByRole('label', {
name: 'rtp_mock_file2.csv',
})

fireEvent.click(csvFileOnRobot)
fireEvent.click(screen.getAllByRole('button')[0])
expect(props.setParameter).toHaveBeenCalledWith(
{
id: '2',
fileName: 'rtp_mock_file2.csv',
},
props.parameter.variableName
)
expect(mockHandleGoBack).toHaveBeenCalled()
})

it.todo('should call a mock function when tapping a csv file')
it('call mock function (setParameter) with filePath + fileName when the selected file is a csv on USB + tapping back button', () => {
render(props)

const csvFileOnUsb = screen.getByRole('label', { name: 'mock-file1.csv' })

fireEvent.click(csvFileOnUsb)
fireEvent.click(screen.getAllByRole('button')[0])
expect(props.setParameter).toHaveBeenCalledWith(
{
filePath: '/media/mock-usb-drive/mock-file1.csv',
fileName: 'mock-file1.csv',
},
props.parameter.variableName
)
expect(mockHandleGoBack).toHaveBeenCalled()
})

it('should render mock empty file component when there is no csv file', () => {
vi.mocked(getShellUpdateDataFiles).mockReturnValue([])
Expand Down
11 changes: 2 additions & 9 deletions app/src/organisms/ProtocolSetupParameters/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import type {
NumberParameter,
RunTimeParameter,
ValueRunTimeParameter,
CsvFileFileType,
} from '@opentrons/shared-data'
import type { LabwareOffsetCreateData } from '@opentrons/api-client'

Expand Down Expand Up @@ -86,14 +87,8 @@ export function ProtocolSetupParameters({
)
const { makeSnackbar } = useToaster()

const csvFileParameter = runTimeParameters.find(
(param): param is CsvFileParameter => param.type === 'csv_file'
)
const initialFileId: string = csvFileParameter?.file?.id ?? ''
const [csvFileInfo, setCSVFileInfo] = React.useState<string>(initialFileId)

const updateParameters = (
value: boolean | string | number,
value: boolean | string | number | CsvFileFileType,
variableName: string
): void => {
const updatedParameters = runTimeParametersOverrides.map(parameter => {
Expand Down Expand Up @@ -289,8 +284,6 @@ export function ProtocolSetupParameters({
}}
parameter={chooseCsvFileScreen}
setParameter={updateParameters}
csvFileInfo={csvFileInfo}
setCsvFileInfo={setCSVFileInfo}
/>
)
}
Expand Down
9 changes: 8 additions & 1 deletion shared-data/js/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,16 @@ interface BooleanParameter extends BaseRunTimeParameter {
value: boolean
}

export interface CsvFileFileType {
id?: string
file?: File | null
filePath?: string
fileName?: string
}

export interface CsvFileParameter extends BaseRunTimeParameter {
type: CsvFileParameterType
file?: { id?: string; file?: File | null } | null
file?: CsvFileFileType | null
}

type NumberParameterType = 'int' | 'float'
Expand Down

0 comments on commit f2ab3d3

Please sign in to comment.