Skip to content
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

fix(protocol-designer): update ListItemDescriptor to align with the latest design #16563

Merged
merged 6 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions components/src/atoms/ListItem/ListItem.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const ListItemDescriptorDefault: Story = {
type: 'noActive',
children: (
<ListItemDescriptor
type="default"
type="large"
content={<div>mock content</div>}
description={<div>mock description</div>}
/>
Expand All @@ -63,7 +63,7 @@ export const ListItemDescriptorMini: Story = {
type: 'noActive',
children: (
<ListItemDescriptor
type="mini"
type="default"
content={<div>mock content</div>}
description={<div>mock description</div>}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,34 @@ import { Flex } from '../../../primitives'
import {
ALIGN_FLEX_START,
DIRECTION_ROW,
FLEX_AUTO,
JUSTIFY_SPACE_BETWEEN,
} from '../../../styles'
import { SPACING } from '../../../ui-style-constants'

interface ListItemDescriptorProps {
type: 'default' | 'mini'
type: 'default' | 'large'
description: JSX.Element | string
content: JSX.Element | string
isInSlideout?: boolean
}

export const ListItemDescriptor = (
props: ListItemDescriptorProps
): JSX.Element => {
const { description, content, type } = props
const { description, content, type, isInSlideout = false } = props
return (
<Flex
flexDirection={DIRECTION_ROW}
gridGap={SPACING.spacing8}
width="100%"
alignItems={ALIGN_FLEX_START}
justifyContent={type === 'mini' ? JUSTIFY_SPACE_BETWEEN : 'none'}
padding={
type === 'mini'
? `${SPACING.spacing4} ${SPACING.spacing8}`
: SPACING.spacing12
}
justifyContent={type === 'default' ? JUSTIFY_SPACE_BETWEEN : 'none'}
padding={type === 'default' ? SPACING.spacing4 : SPACING.spacing12}
>
<Flex
flex={type === 'default' && '1'}
width={type === 'mini' ? FLEX_AUTO : '40%'}
>
<Flex minWidth={isInSlideout ? undefined : '13.75rem'}>
{description}
</Flex>
<Flex flex={type === 'default' && '1.95'} overflowWrap="anywhere">
{content}
</Flex>
<Flex width="100%">{content}</Flex>
</Flex>
)
}
14 changes: 13 additions & 1 deletion protocol-designer/src/atoms/constants.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
import { css } from 'styled-components'
import { COLORS } from '@opentrons/components'
import { COLORS, OVERFLOW_HIDDEN } from '@opentrons/components'
import type { FlattenSimpleInterpolation } from 'styled-components'

export const BUTTON_LINK_STYLE = css`
color: ${COLORS.grey60};
&:hover {
color: ${COLORS.grey40};
}
`

export const LINE_CLAMP_TEXT_STYLE = (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

lineClamp: number
): FlattenSimpleInterpolation => css`
display: -webkit-box;
-webkit-box-orient: vertical;
overflow: ${OVERFLOW_HIDDEN};
text-overflow: ellipsis;
word-wrap: break-word;
-webkit-line-clamp: ${lineClamp};
`
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export function MaterialsListModal({
? fixtures.map(fixture => (
<ListItem type="noActive" key={fixture.id}>
<ListItemDescriptor
type="default"
type="large"
description={
fixture.location != null ? (
<DeckInfoLabel
Expand Down Expand Up @@ -128,7 +128,7 @@ export function MaterialsListModal({
return (
<ListItem type="noActive" key={`hardware${id}`}>
<ListItemDescriptor
type="default"
type="large"
description={
<DeckInfoLabel deckLabel={formatLocation(hw.slot)} />
}
Expand Down Expand Up @@ -187,7 +187,7 @@ export function MaterialsListModal({
return (
<ListItem type="noActive" key={`labware_${lw.id}`}>
<ListItemDescriptor
type="default"
type="large"
description={
<DeckInfoLabel deckLabel={deckLabelSlot} />
}
Expand Down
4 changes: 2 additions & 2 deletions protocol-designer/src/organisms/SlotInformation/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const SlotInformation: React.FC<SlotInformationProps> = ({
{liquids.length > 1 ? (
<ListItem type="noActive">
<ListItemDescriptor
type="mini"
type="default"
content={liquids.join(', ')}
description={t('liquid')}
/>
Expand Down Expand Up @@ -113,7 +113,7 @@ function StackInfo({ title, stackInformation }: StackInfoProps): JSX.Element {
return (
<ListItem type="noActive">
<ListItemDescriptor
type="mini"
type="default"
content={
stackInformation != null ? (
<StyledText
Expand Down
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be overkill, but I am thinking of a more robust way to retrieve labware name on a module better than simply splitting on "on". Maybe:

  const timeline = useSelector(getRobotStateAtActiveItem)
  const labware = timeline?.labware ?? {}

  let labwareOnModuleId: string | null = null
  for (const [id, lw] of Object.entries(labware)) {
    if (lw.slot === formData.moduleId) {
      labwareOnModuleId = id
      break
    }
  }
  const nickname =
    labwareOnModuleId != null
      ? useSelector(getLabwareNicknamesById)[labwareOnModuleId ?? '']
      : null
  const moduleDisplayName = getModuleDisplayName(moduleModel)

can work, and we can use an escape hatch if this returns null. I also think this may be beyond the scope of this PR's intention, and I am happy to address it later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ncdiehl11 I think we can do that in another PR.

Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import { useSelector } from 'react-redux'
import { useTranslation } from 'react-i18next'
import {
COLORS,
DIRECTION_COLUMN,
DeckInfoLabel,
DIRECTION_COLUMN,
Divider,
Flex,
ListItem,
ListItemDescriptor,
SPACING,
StyledText,
} from '@opentrons/components'
Expand All @@ -30,6 +31,7 @@ import {
getModuleEntities,
} from '../../../../../../step-forms/selectors'
import { getModulesOnDeckByType } from '../../../../../../ui/modules/utils'
import { LINE_CLAMP_TEXT_STYLE } from '../../../../../../atoms'

import type { StepFormProps } from '../../types'

Expand Down Expand Up @@ -67,7 +69,6 @@ export function MagnetTools(props: StepFormProps): JSX.Element {
})
: ''
const engageHeightCaption = `${engageHeightMinMax} ${engageHeightDefault}`
// TODO (10-9-2024): Replace ListItem with ListItemDescriptor
return (
<Flex flexDirection={DIRECTION_COLUMN}>
<Flex
Expand All @@ -80,22 +81,31 @@ export function MagnetTools(props: StepFormProps): JSX.Element {
{t('protocol_steps:module')}
</StyledText>
<ListItem type="noActive">
<Flex padding={SPACING.spacing12} gridGap={SPACING.spacing32}>
<Flex>
<DeckInfoLabel deckLabel={slotLocation} />
</Flex>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing4}>
<StyledText desktopStyle="bodyDefaultRegular">
{slotInfo[0]}
</StyledText>
<StyledText
desktopStyle="bodyDefaultRegular"
color={COLORS.grey60}
>
{slotInfo[1]}
</StyledText>
</Flex>
</Flex>
<ListItemDescriptor
type="large"
isInSlideout
content={
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing4}>
<StyledText
desktopStyle="bodyDefaultRegular"
css={LINE_CLAMP_TEXT_STYLE(2)}
>
{slotInfo[0]}
</StyledText>
<StyledText
desktopStyle="bodyDefaultRegular"
color={COLORS.grey60}
>
{slotInfo[1]}
</StyledText>
</Flex>
}
description={
<Flex width="2.875rem">
Copy link
Contributor Author

@koji koji Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this width is a request from Mel (46px)

<DeckInfoLabel deckLabel={slotLocation} />
</Flex>
}
/>
</ListItem>
</Flex>
<Divider marginY="0" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export function InstrumentsInfo({
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing4}>
<ListItem type="noActive" key={`ProtocolOverview_robotType`}>
<ListItemDescriptor
type="default"
type="large"
description={t('robotType')}
content={
robotType === FLEX_ROBOT_TYPE
Expand All @@ -103,22 +103,22 @@ export function InstrumentsInfo({
</ListItem>
<ListItem type="noActive" key={`ProtocolOverview_left`}>
<ListItemDescriptor
type="default"
type="large"
description={t('left_pip')}
content={pipetteInfo(leftPipette)}
/>
</ListItem>
<ListItem type="noActive" key={`ProtocolOverview_right`}>
<ListItemDescriptor
type="default"
type="large"
description={t('right_pip')}
content={pipetteInfo(rightPipette)}
/>
</ListItem>
{robotType === FLEX_ROBOT_TYPE ? (
<ListItem type="noActive" key={`ProtocolOverview_gripper`}>
<ListItemDescriptor
type="default"
type="large"
description={t('extension')}
content={isGripperAttached ? t('gripper') : t('na')}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { useTranslation } from 'react-i18next'
import { css } from 'styled-components'
import {
ALIGN_CENTER,
DIRECTION_COLUMN,
Expand All @@ -8,10 +7,10 @@ import {
LiquidIcon,
ListItem,
ListItemDescriptor,
OVERFLOW_HIDDEN,
SPACING,
StyledText,
} from '@opentrons/components'
import { LINE_CLAMP_TEXT_STYLE } from '../../atoms'

import type { AllIngredGroupFields } from '../../labware-ingred/types'

Expand All @@ -36,15 +35,15 @@ export function LiquidDefinitions({
key={`${liquid.name}_${liquid.displayColor}_${index}`}
>
<ListItemDescriptor
type="default"
type="large"
description={
<Flex alignItems={ALIGN_CENTER} gridGap={SPACING.spacing8}>
<LiquidIcon color={liquid.displayColor} />
<StyledText
desktopStyle="bodyDefaultRegular"
overflowWrap="anywhere"
id="liquid-name"
css={LIQUID_DEFINITION_TEXT}
css={LINE_CLAMP_TEXT_STYLE(3)}
>
{liquid.name}
</StyledText>
Expand All @@ -61,11 +60,3 @@ export function LiquidDefinitions({
</Flex>
)
}

const LIQUID_DEFINITION_TEXT = css`
display: -webkit-box;
-webkit-box-orient: vertical;
-webkit-line-clamp: 3;
overflow: ${OVERFLOW_HIDDEN};
text-overflow: ellipsis;
`
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export function ProtocolMetadata({
return (
<ListItem type="noActive" key={`ProtocolOverview_${title}`}>
<ListItemDescriptor
type="default"
type="large"
description={t(`${title}`)}
content={value ?? t('na')}
/>
Expand All @@ -70,7 +70,7 @@ export function ProtocolMetadata({
})}
<ListItem type="noActive" key="ProtocolOverview_robotVersion">
<ListItemDescriptor
type="default"
type="large"
description={t('required_app_version')}
content={t('app_version', {
version: REQUIRED_APP_VERSION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function StepsInfo({ savedStepForms }: StepsInfoProps): JSX.Element {
) : (
<ListItem type="noActive" key="ProtocolOverview_Step">
<ListItemDescriptor
type="default"
type="large"
description={
<StyledText
desktopStyle="bodyDefaultRegular"
Expand Down
13 changes: 2 additions & 11 deletions protocol-designer/src/pages/ProtocolOverview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
getUnusedTrash,
} from '../../components/FileSidebar/utils'
import { MaterialsListModal } from '../../organisms/MaterialsListModal'
import { BUTTON_LINK_STYLE } from '../../atoms'
import { BUTTON_LINK_STYLE, LINE_CLAMP_TEXT_STYLE } from '../../atoms'
import { getMainPagePortalEl } from '../../components/portals/MainPageModalPortal'
import {
EditProtocolMetadataModal,
Expand Down Expand Up @@ -292,7 +292,7 @@ export function ProtocolOverview(): JSX.Element {
<Flex flex="1">
<StyledText
desktopStyle="displayBold"
css={PROTOCOL_NAME_TEXT_STYLE}
css={LINE_CLAMP_TEXT_STYLE(3)}
>
{protocolName != null && protocolName !== ''
? protocolName
Expand Down Expand Up @@ -418,15 +418,6 @@ export function ProtocolOverview(): JSX.Element {
)
}

const PROTOCOL_NAME_TEXT_STYLE = css`
display: -webkit-box;
-webkit-box-orient: vertical;
overflow: hidden;
text-overflow: ellipsis;
word-wrap: break-word;
-webkit-line-clamp: 3;
`

const MIN_OVERVIEW_WIDTH = '64rem'
const COLUMN_GRID_GAP = '5rem'
const COLUMN_STYLE = css`
Expand Down
Loading