-
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
feat(protocol-designer): make timeline responsive #17109
Conversation
} from '@opentrons/components' | ||
import { TimelineToolbox } from './Timeline/TimelineToolbox' | ||
|
||
const INITIAL_SIDEBAR_WIDTH = 350 |
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.
i'll get the right value from Mel next week
@@ -56,6 +59,7 @@ export function ProtocolSteps(): JSX.Element { | |||
const [deckView, setDeckView] = useState< | |||
typeof leftString | typeof rightString | |||
>(leftString) | |||
const [targetWidth, setTargetWidth] = useState<number>(350) |
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.
the initial value will be updated.
|
Whoa, looks really slick. I do see some highlighting when dragging the timeline toolbox (see "Ending deck"). Not sure if this is blocking, though. Screen.Recording.2024-12-17.at.10.18.12.AM.mov |
protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/StepContainer.tsx
Show resolved
Hide resolved
protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/StepContainer.tsx
Show resolved
Hide resolved
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.
Left some comments but generally looking good
const addStep = (stepType: StepType): ReturnType<any> => | ||
dispatch(stepsActions.addAndSelectStep({ stepType })) | ||
|
||
const items = getSupportedSteps() | ||
.filter(stepType => isStepTypeEnabled[stepType]) | ||
const items = getSupportedSteps(enableComment) |
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.
maybe
const items = getSupportedSteps(enableComment) | |
getSupportedSteps(enableComment). | |
reduce<JSX.Element[]>((acc, stepType) => { | |
return getIsStepTypeEnabled(enableComment, modules)[stepType] | |
? [...acc, <AddOverflowButton ... />] | |
: acc | |
}, []) |
protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/AddStepButton.tsx
Outdated
Show resolved
Hide resolved
white-space: ${NO_WRAP}; | ||
bottom: 4.2rem; | ||
border-radius: ${BORDERS.borderRadius8}; | ||
box-shadow: 0px 1px 3px rgba(0, 0, 0, 0.2); |
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.
Do we have a constant for this?
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.
you mean box-shadow or the entire style?
i think we don't have the constant for this part.
protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/StepContainer.tsx
Outdated
Show resolved
Hide resolved
protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/TerminalItemStep.tsx
Outdated
Show resolved
Hide resolved
protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/TerminalItemStep.tsx
Outdated
Show resolved
Hide resolved
@ncdiehl11 thank you for catching this. |
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.
left one comment but looks good 💪
yeah, that is Mel's current design. |
Overview
make timeline responsive
close AUTH-877
Test Plan and Hands on Testing
Changelog
Review requests
Risk assessment
low