Skip to content

Commit

Permalink
feat(protocol-designer): clean up navigation and modal hierarchy (#2638)
Browse files Browse the repository at this point in the history
Partition views within tabs, so that forms and modals do not appear outside of their home tab.
Properly restrict tab navigation, when in blocking state(e.g. analytics modal, splash). Preserve
form state when navigating between liquids and design tabs.

Closes #2198
  • Loading branch information
b-cooper authored Nov 8, 2018
1 parent 5e86537 commit 134558f
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 52 deletions.
7 changes: 0 additions & 7 deletions protocol-designer/src/components/ProtocolEditor.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
// @flow
import * as React from 'react'

import TimelineAlerts from '../components/alerts/TimelineAlerts'
import Hints from '../components/Hints'
import ConnectedMoreOptionsModal from '../containers/ConnectedMoreOptionsModal'
import ConnectedNav from '../containers/ConnectedNav'
import StepEditForm from '../components/StepEditForm'
import ConnectedSidebar from '../containers/ConnectedSidebar'
import ConnectedTitleBar from '../containers/ConnectedTitleBar'
import ConnectedMainPanel from '../containers/ConnectedMainPanel'
Expand All @@ -31,8 +28,6 @@ export default function ProtocolEditor () {
<ConnectedSidebar />
<div className={styles.main_page_wrapper}>
<ConnectedTitleBar />
<TimelineAlerts />
<Hints />

<div className={styles.main_page_content}>
<AnalyticsModal />
Expand All @@ -42,8 +37,6 @@ export default function ProtocolEditor () {
{/* TODO: Ian 2018-06-28 All main page modals will go here */}
<MainPageModalPortalRoot />

<StepEditForm />

<ConnectedMainPanel />
</div>
</div>
Expand Down
17 changes: 9 additions & 8 deletions protocol-designer/src/components/StepEditForm/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,27 @@ type StepEditFormState = {

type Props = SP & DP

// TODO: type fieldNames, don't use `any`
const getDirtyFields = (isNewStep: ?boolean, formData: ?FormData): Array<any> => (
isNewStep ? [] : without(Object.keys(formData || {}), 'stepType', 'id')
)

class StepEditForm extends React.Component<Props, StepEditFormState> {
constructor (props: Props) {
super(props)
const {isNewStep, formData} = props
this.state = {
showConfirmDeleteModal: false,
focusedField: null,
dirtyFields: [], // TODO: initialize to dirty if not new form
dirtyFields: getDirtyFields(isNewStep, formData),
}
}

componentDidUpdate (prevProps) {
// NOTE: formData is sometimes undefined between steps
if (get(this.props, 'formData.id') !== get(prevProps, 'formData.id')) {
if (this.props.isNewStep) {
this.setState({ focusedField: null, dirtyFields: [] })
} else {
// TODO: type fieldNames, don't use `any`
const fieldNames: Array<any> = without(Object.keys(this.props.formData || {}), 'stepType', 'id')
this.setState({focusedField: null, dirtyFields: fieldNames})
}
const {isNewStep, formData} = this.props
this.setState({ focusedField: null, dirtyFields: getDirtyFields(isNewStep, formData) })
}
}

Expand Down
23 changes: 13 additions & 10 deletions protocol-designer/src/containers/ConnectedDeckSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ import {Deck, ClickOutside} from '@opentrons/components'
import styles from './Deck.css'
import i18n from '../localization'

import {Portal} from '../components/portals/MainPageModalPortal'
import BrowseLabwareModal from '../components/labware/BrowseLabwareModal'
import Hints from '../components/Hints'
import LiquidPlacementModal from '../components/LiquidPlacementModal.js'
import LabwareContainer from '../containers/LabwareContainer.js'
import LabwareSelectionModal from '../components/LabwareSelectionModal'
import BrowseLabwareModal from '../components/labware/BrowseLabwareModal'
import StepEditForm from '../components/StepEditForm'
import TimelineAlerts from '../components/alerts/TimelineAlerts'

import {selectors} from '../labware-ingred/reducers'
import * as actions from '../labware-ingred/actions'
Expand Down Expand Up @@ -87,22 +91,21 @@ class DeckSetup extends React.Component<Props> {
}

render () {
if (this.props.selectedTerminalItemId !== START_TERMINAL_ITEM_ID) {
// Temporary quickfix: if we're not in deck setup mode,
// hide the labware dropdown and ingredient selection modal
// and just show the deck.
// TODO Ian 2018-05-30 this shouldn't be a responsibility of DeckSetup
return this.renderDeck()
}
const startTerminalItemSelected = this.props.selectedTerminalItemId === START_TERMINAL_ITEM_ID

// NOTE: besides `Deck`, these are all modal-like components that show up
// only when user is on deck setup / ingred selection "page".
// Once DeckSetup is broken apart and moved into ProtocolEditor,
// this will go away
return (
<React.Fragment>
<LabwareSelectionModal />
{this.props.ingredSelectionMode && <LiquidPlacementModal />}
<Portal>
<TimelineAlerts />
<Hints />
{startTerminalItemSelected && <LabwareSelectionModal />}
{!startTerminalItemSelected && <StepEditForm />}
{startTerminalItemSelected && this.props.ingredSelectionMode && <LiquidPlacementModal />}
</Portal>
{this.renderDeck()}
</React.Fragment>
)
Expand Down
1 change: 1 addition & 0 deletions protocol-designer/src/containers/ConnectedNav.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ function Nav (props: Props) {
<NavTab
iconName='settings'
title={i18n.t('nav.tab_name.settings')}
disabled={props.currentPage === 'file-splash'}
selected={props.currentPage === 'settings-privacy'}
onClick={props.handleClick('settings-privacy')} />
</React.Fragment>
Expand Down
34 changes: 18 additions & 16 deletions protocol-designer/src/containers/ConnectedTitleBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type {BaseState} from '../types'

type Props = React.ElementProps<typeof TitleBar>
type DP = { onBackClick: $PropertyType<Props, 'onBackClick'> }
type SP = $Diff<Props, DP> & {_page: Page}
type SP = $Diff<Props, DP> & {_page: Page, _liquidPlacementMode?: boolean}

type TitleWithIconProps = {
iconName?: ?IconName,
Expand All @@ -50,6 +50,7 @@ function mapStateToProps (state: BaseState): SP {
const labwareNames = labwareIngredSelectors.getLabwareNames(state)
const labwareNickname = labware && labware.id && labwareNames[labware.id]
const drilledDownLabwareId = labwareIngredSelectors.getDrillDownLabwareId(state)
const liquidPlacementMode = !!labwareIngredSelectors.getSelectedContainer(state)

switch (_page) {
case 'liquids':
Expand All @@ -62,14 +63,6 @@ function mapStateToProps (state: BaseState): SP {
title: i18n.t([`nav.title.${_page}`, fileName]),
subtitle: i18n.t([`nav.subtitle.${_page}`, '']),
}
case 'ingredient-detail': {
return {
_page,
title: labwareNickname || null,
subtitle: labware && humanizeLabwareType(labware.type),
backButtonLabel: 'Deck',
}
}
case 'well-selection-modal':
return {
_page,
Expand All @@ -83,6 +76,15 @@ function mapStateToProps (state: BaseState): SP {
default: {
// NOTE: this default case error should never be reached, it's just a sanity check
if (_page !== 'steplist') console.error('ConnectedTitleBar got an unsupported page, returning steplist instead')
if (liquidPlacementMode) {
return {
_page,
_liquidPlacementMode: liquidPlacementMode,
title: labwareNickname || null,
subtitle: labware && humanizeLabwareType(labware.type),
backButtonLabel: 'Deck',
}
}
let subtitle
let backButtonLabel
let title
Expand All @@ -105,23 +107,23 @@ function mapStateToProps (state: BaseState): SP {
}

function mergeProps (stateProps: SP, dispatchProps: {dispatch: Dispatch<*>}): Props {
const {_page, ...props} = stateProps
const {_page, _liquidPlacementMode, ...props} = stateProps
const {dispatch} = dispatchProps

let onBackClick

if (_page === 'ingredient-detail') {
onBackClick = () => dispatch(closeIngredientSelector())
if (_page === 'steplist') {
if (_liquidPlacementMode) {
onBackClick = () => dispatch(closeIngredientSelector())
} else if (props.backButtonLabel) {
onBackClick = () => {}
}
}

if (_page === 'well-selection-modal') {
onBackClick = () => dispatch(closeWellSelectionModal())
}

if (_page === 'steplist' && props.backButtonLabel) {
onBackClick = () => {}
}

return {
...props,
onBackClick,
Expand Down
1 change: 0 additions & 1 deletion protocol-designer/src/labware-ingred/reducers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ const selectedLiquidGroup = handleActions({
DESELECT_LIQUID_GROUP: () => unselectedLiquidGroupState,
CREATE_NEW_LIQUID_GROUP_FORM: (): SelectedLiquidGroupState =>
({liquidGroupId: null, newLiquidGroup: true}),
NAVIGATE_TO_PAGE: () => unselectedLiquidGroupState, // clear selection on navigate
EDIT_LIQUID_GROUP: () => unselectedLiquidGroupState, // clear on form save
}, unselectedLiquidGroupState)

Expand Down
10 changes: 0 additions & 10 deletions protocol-designer/src/navigation/selectors.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
// @flow
import type {BaseState, Selector} from '../types'
// import {createSelector} from 'reselect'

import {rootSelector as navigationRootSelector} from './reducers'
import {selectors as labwareIngredSelectors} from '../labware-ingred/reducers'
import wellSelectionSelectors from '../well-selection/selectors'
// import {selectors as steplistSelectors} from '../steplist'

import type {Page} from './types'

Expand All @@ -15,10 +10,5 @@ export const newProtocolModal = (state: BaseState) =>
export const currentPage: Selector<Page> = (state: BaseState) => {
let page = navigationRootSelector(state).page

if (wellSelectionSelectors.wellSelectionModalData(state)) {
page = 'well-selection-modal'
} else if (labwareIngredSelectors.getSelectedContainer(state)) {
page = 'ingredient-detail'
}
return page
}

0 comments on commit 134558f

Please sign in to comment.