From d19d29b9f48a454d44624dd32a6f6bf1bc9776f9 Mon Sep 17 00:00:00 2001 From: Ian London Date: Fri, 3 Aug 2018 15:07:40 -0400 Subject: [PATCH] fix(protocol-designer): fix serialized name in ingred list (#2002) - factor apart utils - refactor + write tests for sortWells fn - use sortWells where missing - remove vestigal serializedName code Closes #1294 --- .../components/IngredientPropertiesForm.css | 2 +- .../components/IngredientPropertiesForm.js | 19 +++------- .../src/components/IngredientsList.js | 10 +++-- .../src/components/WellToolTip.js | 6 +-- .../__tests__/ingredients.test.js | 2 - .../__tests__/selectors.test.js | 3 +- .../src/labware-ingred/reducers/index.js | 7 ++-- protocol-designer/src/labware-ingred/types.js | 3 +- .../__tests__/wellContentsAllLabware.test.js | 3 +- .../src/{utils.js => utils/index.js} | 38 ++++++++++++++++++- .../src/utils/test/sortWells.test.js | 26 +++++++++++++ .../src/well-selection/selectors.js | 31 +-------------- 12 files changed, 85 insertions(+), 65 deletions(-) rename protocol-designer/src/{utils.js => utils/index.js} (74%) create mode 100644 protocol-designer/src/utils/test/sortWells.test.js diff --git a/protocol-designer/src/components/IngredientPropertiesForm.css b/protocol-designer/src/components/IngredientPropertiesForm.css index 2233f637ecf..b9839a95144 100644 --- a/protocol-designer/src/components/IngredientPropertiesForm.css +++ b/protocol-designer/src/components/IngredientPropertiesForm.css @@ -18,7 +18,7 @@ lost-column: 4/16; } -.serialize_name { +.individualize { lost-column: 6/16; } diff --git a/protocol-designer/src/components/IngredientPropertiesForm.js b/protocol-designer/src/components/IngredientPropertiesForm.js index 5eb5795c4d8..ead445e7991 100644 --- a/protocol-designer/src/components/IngredientPropertiesForm.js +++ b/protocol-designer/src/components/IngredientPropertiesForm.js @@ -112,8 +112,7 @@ class IngredientPropertiesForm extends React.Component { name: null, volume: null, description: null, - individualize: false, - serializeName: null + individualize: false }, commonIngredGroupId: null } @@ -144,7 +143,7 @@ class IngredientPropertiesForm extends React.Component { const allIngredientGroupFields = (nextIngredGroupFields || this.props.allIngredientGroupFields || {}) if (ingredGroupId && ingredGroupId in allIngredientGroupFields) { - const {name, volume, description, individualize, serializeName} = this.state.input + const {name, volume, description, individualize} = this.state.input const newIngredFields = allIngredientGroupFields[ingredGroupId] this.setState({ ...this.state, @@ -152,8 +151,7 @@ class IngredientPropertiesForm extends React.Component { name: newIngredFields.name || name, volume: newIngredFields.volume || volume, description: newIngredFields.description || description, - individualize: newIngredFields.individualize || individualize, - serializeName: newIngredFields.serializeName || serializeName + individualize: newIngredFields.individualize || individualize } }, cb) } else { @@ -168,8 +166,7 @@ class IngredientPropertiesForm extends React.Component { name: null, volume: null, description: null, - individualize: false, - serializeName: null + individualize: false } }, cb) } @@ -293,19 +290,13 @@ class IngredientPropertiesForm extends React.Component { - {/* TODO: Ian 2018-06-01 remove all remnants of this text field see issue #1294 - {individualize && } */} {showIngredientDropdown && diff --git a/protocol-designer/src/components/IngredientsList.js b/protocol-designer/src/components/IngredientsList.js index 7c9a85391be..c90a35c5e3c 100644 --- a/protocol-designer/src/components/IngredientsList.js +++ b/protocol-designer/src/components/IngredientsList.js @@ -3,6 +3,7 @@ import React from 'react' import {IconButton, SidePanel, swatchColors} from '@opentrons/components' +import {sortWells} from '../utils' import {PDTitledList, PDListItem} from './lists' import stepItemStyles from './steplist/StepItem.css' import StepDescription from './StepDescription' @@ -48,11 +49,12 @@ class IngredGroupCard extends React.Component { groupId, labwareWellContents } = this.props - const {serializeName, individualize, description, name} = ingredGroup + const {individualize, description, name} = ingredGroup const {isExpanded} = this.state - const wellsWithIngred = Object.keys(labwareWellContents).filter(well => - labwareWellContents[well][groupId]) + const wellsWithIngred = Object.keys(labwareWellContents) + .sort(sortWells) + .filter(well => labwareWellContents[well][groupId]) if (wellsWithIngred.length < 1) { // do not show ingred card if it has no instances for this labware @@ -89,7 +91,7 @@ class IngredGroupCard extends React.Component { return {wellContent.individualize &&
- {wellContent.serializeName || 'Sample'} {wellContent.ingredientNum} + {wellContent.name || ''} {wellContent.ingredientNum}
}
{wellContent.volume} uL diff --git a/protocol-designer/src/labware-ingred/__tests__/ingredients.test.js b/protocol-designer/src/labware-ingred/__tests__/ingredients.test.js index 6cc3c93ea4a..176db6ee07f 100644 --- a/protocol-designer/src/labware-ingred/__tests__/ingredients.test.js +++ b/protocol-designer/src/labware-ingred/__tests__/ingredients.test.js @@ -166,7 +166,6 @@ describe.skip('COPY_LABWARE action', () => { describe('EDIT_INGREDIENT action', () => { const ingredFields = { name: 'Cool Ingredient', - serializeName: null, volume: 250, description: 'far out!', individualize: false @@ -211,7 +210,6 @@ describe('EDIT_INGREDIENT action', () => { type: 'EDIT_INGREDIENT', payload: { name: 'Cool Ingredient', - serializeName: false, volume: 250, description: 'far out!', individualize: false, diff --git a/protocol-designer/src/labware-ingred/__tests__/selectors.test.js b/protocol-designer/src/labware-ingred/__tests__/selectors.test.js index f7a8ab4ba55..6c0803c0782 100644 --- a/protocol-designer/src/labware-ingred/__tests__/selectors.test.js +++ b/protocol-designer/src/labware-ingred/__tests__/selectors.test.js @@ -6,8 +6,7 @@ const baseIngredFields = { groupId: '0', name: 'Some Ingred', description: null, - individualize: false, - serializeName: null + individualize: false } const allIngredientsXXSingleIngred = { diff --git a/protocol-designer/src/labware-ingred/reducers/index.js b/protocol-designer/src/labware-ingred/reducers/index.js index e7c6f5a1375..8403ac6eaa3 100644 --- a/protocol-designer/src/labware-ingred/reducers/index.js +++ b/protocol-designer/src/labware-ingred/reducers/index.js @@ -11,7 +11,7 @@ import reduce from 'lodash/reduce' import isEmpty from 'lodash/isEmpty' import {sortedSlotnames, FIXED_TRASH_ID} from '../../constants.js' -import {uuid} from '../../utils.js' +import {uuid} from '../../utils' import type {DeckSlot} from '@opentrons/components' @@ -181,12 +181,11 @@ export const savedLabware = handleActions({ type IngredientsState = IngredientGroups export const ingredients = handleActions({ EDIT_INGREDIENT: (state, action: EditIngredient) => { - const {groupId, description, individualize, name, serializeName} = action.payload + const {groupId, description, individualize, name} = action.payload const ingredFields: IngredientInstance = { description, individualize, - name, - serializeName + name } return { diff --git a/protocol-designer/src/labware-ingred/types.js b/protocol-designer/src/labware-ingred/types.js index 8b96e8da01d..83828be1fc8 100644 --- a/protocol-designer/src/labware-ingred/types.js +++ b/protocol-designer/src/labware-ingred/types.js @@ -43,8 +43,7 @@ export type IngredInputs = { name: string | null, volume: number | null, description: string | null, - individualize: boolean, - serializeName: string | null + individualize: boolean } export type IngredInputFields = $Exact diff --git a/protocol-designer/src/top-selectors/well-contents/__tests__/wellContentsAllLabware.test.js b/protocol-designer/src/top-selectors/well-contents/__tests__/wellContentsAllLabware.test.js index d037fcd38f8..443802591d7 100644 --- a/protocol-designer/src/top-selectors/well-contents/__tests__/wellContentsAllLabware.test.js +++ b/protocol-designer/src/top-selectors/well-contents/__tests__/wellContentsAllLabware.test.js @@ -5,8 +5,7 @@ const baseIngredFields = { groupId: '0', name: 'Some Ingred', description: null, - individualize: false, - serializeName: null + individualize: false } const containerState = { diff --git a/protocol-designer/src/utils.js b/protocol-designer/src/utils/index.js similarity index 74% rename from protocol-designer/src/utils.js rename to protocol-designer/src/utils/index.js index 6a60350593b..60af8c3ddb9 100644 --- a/protocol-designer/src/utils.js +++ b/protocol-designer/src/utils/index.js @@ -1,8 +1,8 @@ // @flow import uuidv1 from 'uuid/v1' import {wellNameSplit} from '@opentrons/components' -import type {BoundingRect, GenericRect} from './collision-types' -import type {Wells} from './labware-ingred/types' +import type {BoundingRect, GenericRect} from '../collision-types' +import type {Wells} from '../labware-ingred/types' export type FormConnectorFactory = ( handleChange: (accessor: F) => (e: SyntheticInputEvent<*>) => mixed, @@ -101,3 +101,37 @@ export const getCollidingWells = (rectPositions: GenericRect, selectableClassnam return collidedWellData } + +function _parseWell (well: string): ([string, number]) { + const res = well.match(/([A-Z]+)(\d+)/) + const letters = res && res[1] + const number = res && parseInt(res[2]) + + if (!letters || number == null || Number.isNaN(number)) { + console.warn(`Could not parse well ${well}. Got letters: "${letters || 'void'}", number: "${number || 'void'}"`) + return ['', NaN] + } + + return [letters, number] +} + +// TODO: Ian 2018-08-03 use well ordering in shared-data? +/** A compareFunction for sorting an array of well names + * Goes down the columns (A1 to H1 on 96 plate) + * Then L to R across rows (1 to 12 on 96 plate) + */ +export function sortWells (a: string, b: string): number { + const [letterA, numberA] = _parseWell(a) + const [letterB, numberB] = _parseWell(b) + + if (numberA !== numberB) { + return (numberA > numberB) ? 1 : -1 + } + + if (letterA.length !== letterB.length) { + // Eg 'B' vs 'AA' + return (letterA.length > letterB.length) ? 1 : -1 + } + + return (letterA > letterB) ? 1 : -1 +} diff --git a/protocol-designer/src/utils/test/sortWells.test.js b/protocol-designer/src/utils/test/sortWells.test.js new file mode 100644 index 00000000000..72061af652b --- /dev/null +++ b/protocol-designer/src/utils/test/sortWells.test.js @@ -0,0 +1,26 @@ +// @flow +import {sortWells} from '..' + +describe('sortWells', () => { + test('single letters', () => { + const input = ['A12', 'A2', 'B1', 'B12', 'A1'] + const expected = [ + 'A1', 'B1', + 'A2', + 'A12', 'B12' + ] + const result = [...input].sort(sortWells) + expect(result).toEqual(expected) + }) + + // just in case we get a 1536-well plate + test('double letters', () => { + const input = ['AB12', 'A12', 'B12', 'Z12', 'AA12', 'AB1', 'Z1', 'A1', 'B1', 'AA1'] + const expected = [ + 'A1', 'B1', 'Z1', 'AA1', 'AB1', + 'A12', 'B12', 'Z12', 'AA12', 'AB12' + ] + const result = [...input].sort(sortWells) + expect(result).toEqual(expected) + }) +}) diff --git a/protocol-designer/src/well-selection/selectors.js b/protocol-designer/src/well-selection/selectors.js index 5993cdbd21b..e047a5a4b26 100644 --- a/protocol-designer/src/well-selection/selectors.js +++ b/protocol-designer/src/well-selection/selectors.js @@ -2,6 +2,7 @@ import {createSelector} from 'reselect' import reduce from 'lodash/reduce' import {wellSetToWellObj} from './utils' +import {sortWells} from '../utils' import {computeWellAccess} from '@opentrons/shared-data' import type {BaseState, Selector} from '../types' import type {Wells} from '../labware-ingred/types' @@ -59,35 +60,7 @@ const getHighlightedWells: Selector = createSelector( const selectedWellNames: Selector> = createSelector( (state: BaseState) => rootSelector(state).selectedWells.selected, - // TODO LATER Ian 2018-04-19 this will do different sort orders - // depending on selected well ordering (row-wise R2L top to bottom, etc) - // PS don't forget multi-letter wells like 'AA11' (1536-well) - selectedWells => Object.keys(selectedWells).sort((a, b) => { - function parseWell (well: string): ([string, number]) { - const letterMatches = well.match(/\D+/) - const numberMatches = well.match(/\d+/) - - return [ - (letterMatches && letterMatches[0]) || '', - (numberMatches && numberMatches[0] && parseInt(numberMatches[0])) || NaN - ] - } - - const [letterA, numberA] = parseWell(a) - const [letterB, numberB] = parseWell(b) - - // this sort is top to bottom (down column), then left to right (across row) - if (numberA !== numberB) { - return (numberA > numberB) ? 1 : -1 - } - - if (letterA.length !== letterB.length) { - // Eg 'B' vs 'AA' - return (letterA.length > letterB.length) ? 1 : -1 - } - - return (letterA > letterB) ? 1 : -1 - }) + selectedWells => Object.keys(selectedWells).sort(sortWells) ) export default {