-
Notifications
You must be signed in to change notification settings - Fork 178
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(protocol-designer): refactor well order modal to remove single edit dependencies #7375
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
// @flow | ||
import * as React from 'react' | ||
import cx from 'classnames' | ||
import { connect } from 'react-redux' | ||
|
||
import { i18n } from '../../../../localization' | ||
import { Portal } from '../../../portals/MainPageModalPortal' | ||
import { | ||
|
@@ -13,10 +11,7 @@ import { | |
DropdownField, | ||
} from '@opentrons/components' | ||
import modalStyles from '../../../modals/modal.css' | ||
import { actions } from '../../../../steplist' | ||
import { selectors as stepFormSelectors } from '../../../../step-forms' | ||
import type { BaseState, ThunkDispatch } from '../../../../types' | ||
import type { WellOrderOption } from '../../../../form-types' | ||
import type { WellOrderOption, FormData } from '../../../../form-types' | ||
|
||
import { WellOrderViz } from './WellOrderViz' | ||
import styles from './WellOrderInput.css' | ||
|
@@ -30,66 +25,73 @@ const WELL_ORDER_VALUES: Array<WellOrderOption> = [ | |
...VERTICAL_VALUES, | ||
...HORIZONTAL_VALUES, | ||
] | ||
|
||
type SP = {| | ||
initialFirstValue: ?WellOrderOption, | ||
initialSecondValue: ?WellOrderOption, | ||
|} | ||
|
||
type DP = {| | ||
updateValues: ( | ||
firstValue: ?WellOrderOption, | ||
secondValue: ?WellOrderOption | ||
) => mixed, | ||
|} | ||
|
||
type OP = {| | ||
type Props = {| | ||
isOpen: boolean, | ||
closeModal: () => mixed, | ||
prefix: 'aspirate' | 'dispense' | 'mix', | ||
formData: FormData, | ||
updateValues: (firstValue: ?WellOrderOption, ?WellOrderOption) => void, | ||
|} | ||
|
||
type Props = {| ...OP, ...SP, ...DP |} | ||
|
||
type State = { | ||
firstValue: ?WellOrderOption, | ||
secondValue: ?WellOrderOption, | ||
} | ||
|
||
class WellOrderModalComponent extends React.Component<Props, State> { | ||
export class WellOrderModal extends React.Component<Props, State> { | ||
Comment on lines
-59
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thought about refactoring this to be a function component and use hooks but got lazy. I'm down to do that though if we think it's important There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nah we have enough to do for this project 😛 |
||
constructor(props: Props) { | ||
super(props) | ||
const { | ||
initialFirstValue, | ||
initialSecondValue, | ||
} = this.getInitialFirstValues() | ||
this.state = { | ||
firstValue: props.initialFirstValue, | ||
secondValue: props.initialSecondValue, | ||
firstValue: initialFirstValue, | ||
secondValue: initialSecondValue, | ||
} | ||
} | ||
|
||
getInitialFirstValues: () => {| | ||
initialFirstValue: ?WellOrderOption, | ||
initialSecondValue: ?WellOrderOption, | ||
|} = () => { | ||
const { formData, prefix } = this.props | ||
return { | ||
initialFirstValue: formData && formData[`${prefix}_wellOrder_first`], | ||
initialSecondValue: formData && formData[`${prefix}_wellOrder_second`], | ||
} | ||
} | ||
applyChanges = () => { | ||
applyChanges: () => void = () => { | ||
this.props.updateValues(this.state.firstValue, this.state.secondValue) | ||
} | ||
handleReset = () => { | ||
handleReset: () => void = () => { | ||
this.setState( | ||
{ firstValue: DEFAULT_FIRST, secondValue: DEFAULT_SECOND }, | ||
this.applyChanges | ||
) | ||
this.props.closeModal() | ||
} | ||
handleCancel = () => { | ||
const { initialFirstValue, initialSecondValue } = this.props | ||
handleCancel: () => void = () => { | ||
const { | ||
initialFirstValue, | ||
initialSecondValue, | ||
} = this.getInitialFirstValues() | ||
this.setState( | ||
{ firstValue: initialFirstValue, secondValue: initialSecondValue }, | ||
this.applyChanges | ||
) | ||
this.props.closeModal() | ||
} | ||
handleDone = () => { | ||
handleDone: () => void = () => { | ||
this.applyChanges() | ||
this.props.closeModal() | ||
} | ||
makeOnChange = (ordinality: 'first' | 'second') => ( | ||
e: SyntheticEvent<HTMLSelectElement> | ||
) => { | ||
const { value } = e.currentTarget | ||
makeOnChange: ( | ||
ordinality: 'first' | 'second' | ||
) => ( | ||
event: SyntheticEvent<HTMLSelectElement> | ||
) => void = ordinality => event => { | ||
const { value } = event.currentTarget | ||
let nextState = { [`${ordinality}Value`]: value } | ||
if (ordinality === 'first') { | ||
if ( | ||
|
@@ -106,14 +108,18 @@ class WellOrderModalComponent extends React.Component<Props, State> { | |
} | ||
this.setState(nextState) | ||
} | ||
isSecondOptionDisabled = (value: WellOrderOption) => { | ||
isSecondOptionDisabled: WellOrderOption => boolean = ( | ||
value: WellOrderOption | ||
) => { | ||
if (VERTICAL_VALUES.includes(this.state.firstValue)) { | ||
return VERTICAL_VALUES.includes(value) | ||
} else if (HORIZONTAL_VALUES.includes(this.state.firstValue)) { | ||
return HORIZONTAL_VALUES.includes(value) | ||
} else { | ||
return false | ||
} | ||
} | ||
render() { | ||
render(): React.Node { | ||
if (!this.props.isOpen) return null | ||
const { firstValue, secondValue } = this.state | ||
return ( | ||
|
@@ -202,38 +208,3 @@ class WellOrderModalComponent extends React.Component<Props, State> { | |
) | ||
} | ||
} | ||
|
||
const mapSTP = (state: BaseState, ownProps: OP): SP => { | ||
const formData = stepFormSelectors.getUnsavedForm(state) | ||
return { | ||
initialFirstValue: | ||
formData && formData[`${ownProps.prefix}_wellOrder_first`], | ||
initialSecondValue: | ||
formData && formData[`${ownProps.prefix}_wellOrder_second`], | ||
} | ||
} | ||
|
||
const mapDTP = (dispatch: ThunkDispatch<*>, ownProps: OP): DP => ({ | ||
updateValues: (firstValue, secondValue) => { | ||
dispatch( | ||
actions.changeFormInput({ | ||
update: { | ||
[`${ownProps.prefix}_wellOrder_first`]: firstValue, | ||
[`${ownProps.prefix}_wellOrder_second`]: secondValue, | ||
}, | ||
}) | ||
) | ||
}, | ||
}) | ||
|
||
export const WellOrderModal: React.AbstractComponent<OP> = connect< | ||
Props, | ||
OP, | ||
SP, | ||
DP, | ||
_, | ||
_ | ||
>( | ||
mapSTP, | ||
mapDTP | ||
)(WellOrderModalComponent) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,26 @@ | ||
// @flow | ||
import * as React from 'react' | ||
import { connect } from 'react-redux' | ||
import { FormGroup, Tooltip, useHoverTooltip } from '@opentrons/components' | ||
import cx from 'classnames' | ||
import { i18n } from '../../../../localization' | ||
import { selectors as stepFormSelectors } from '../../../../step-forms' | ||
import ZIG_ZAG_IMAGE from '../../../../images/zig_zag_icon.svg' | ||
import { WellOrderModal } from './WellOrderModal' | ||
|
||
import type { BaseState } from '../../../../types' | ||
|
||
import stepEditStyles from '../../StepEditForm.css' | ||
import styles from './WellOrderInput.css' | ||
import type { FormData } from '../../../../form-types' | ||
import type { FieldProps } from '../../types' | ||
|
||
type OP = {| | ||
type Props = {| | ||
className?: ?string, | ||
label?: string, | ||
prefix: 'aspirate' | 'dispense' | 'mix', | ||
formData: FormData, | ||
updateFirstWellOrder: $PropertyType<FieldProps, 'updateValue'>, | ||
updateSecondWellOrder: $PropertyType<FieldProps, 'updateValue'>, | ||
Comment on lines
+18
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. had to do this because this component needs two updaters (first well order and second). |
||
|} | ||
|
||
type SP = {| iconClassNames: Array<string> |} | ||
|
||
type Props = { ...OP, ...SP } | ||
|
||
function WellOrderInput(props: Props) { | ||
export const WellOrderField = (props: Props): React.Node => { | ||
const { formData, updateFirstWellOrder, updateSecondWellOrder } = props | ||
const [isModalOpen, setModalOpen] = React.useState(false) | ||
|
||
const handleOpen = () => { | ||
|
@@ -33,6 +30,21 @@ function WellOrderInput(props: Props) { | |
setModalOpen(false) | ||
} | ||
|
||
const updateValues = (firstValue, secondValue) => { | ||
updateFirstWellOrder(firstValue) | ||
updateSecondWellOrder(secondValue) | ||
} | ||
|
||
const getIconClassNames = () => { | ||
let iconClassNames = [] | ||
if (formData) { | ||
const first = formData[`${props.prefix}_wellOrder_first`] | ||
const second = formData[`${props.prefix}_wellOrder_second`] | ||
iconClassNames = [styles[`${first}_first`], styles[`${second}_second`]] | ||
} | ||
return iconClassNames | ||
} | ||
|
||
const [targetProps, tooltipProps] = useHoverTooltip() | ||
|
||
const className = cx(props.className, { | ||
|
@@ -51,39 +63,20 @@ function WellOrderInput(props: Props) { | |
prefix={props.prefix} | ||
closeModal={handleClose} | ||
isOpen={isModalOpen} | ||
formData={formData} | ||
updateValues={updateValues} | ||
/> | ||
<img | ||
onClick={handleOpen} | ||
src={ZIG_ZAG_IMAGE} | ||
className={cx( | ||
styles.well_order_icon, | ||
{ [styles.icon_with_label]: props.label }, | ||
...props.iconClassNames | ||
getIconClassNames() | ||
)} | ||
/> | ||
</FormGroup> | ||
</div> | ||
</> | ||
) | ||
} | ||
|
||
const mapSTP = (state: BaseState, ownProps: OP): SP => { | ||
const formData = stepFormSelectors.getUnsavedForm(state) | ||
|
||
let iconClassNames = [] | ||
if (formData) { | ||
const first = formData[`${ownProps.prefix}_wellOrder_first`] | ||
const second = formData[`${ownProps.prefix}_wellOrder_second`] | ||
iconClassNames = [styles[`${first}_first`], styles[`${second}_second`]] | ||
} | ||
return { iconClassNames } | ||
} | ||
|
||
export const WellOrderField: React.AbstractComponent<OP> = connect< | ||
Props, | ||
OP, | ||
SP, | ||
_, | ||
_, | ||
_ | ||
>(mapSTP)(WellOrderInput) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// @flow | ||
import React from 'react' | ||
import { mount } from 'enzyme' | ||
import { act } from 'react-dom/test-utils' | ||
import { WellOrderField } from '../WellOrderField' | ||
import { WellOrderModal } from '../WellOrderField/WellOrderModal' | ||
|
||
describe('WellOrderField', () => { | ||
const render = _props => mount(<WellOrderField {..._props} />) | ||
|
||
let props | ||
beforeEach(() => { | ||
props = { | ||
prefix: 'aspirate', | ||
formData: ({}: any), | ||
updateFirstWellOrder: jest.fn(), | ||
updateSecondWellOrder: jest.fn(), | ||
} | ||
}) | ||
|
||
describe('WellOrderModal', () => { | ||
it('should call correct updater fns passed in', () => { | ||
const wrapper = render(props) | ||
const wellOrderModal = wrapper.find(WellOrderModal) | ||
act(() => { | ||
wellOrderModal.prop('updateValues')('l2r', 't2b') | ||
}) | ||
expect(props.updateFirstWellOrder).toHaveBeenCalledWith('l2r') | ||
expect(props.updateSecondWellOrder).toHaveBeenCalledWith('t2b') | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be like
firstValue: ?WellOrderOption, secondValue: ?WellOrderOption
? I didn't know syntax would allow you to do just one named arg. Just a readability thing thoughThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol i didn't even realize i did that — good catch will fix this real quick