refactor(app,labware-library): Migrate some more components to positionMode="passThrough"
#20302
+56
−39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
koji
approved these changes
Dec 8, 2025
No further changes needed for this one. This labware thumbnail was already using getLabwareViewBox(), so passThrough was correct.
This one is tricky for a few reasons. 1. We're hard-coding an SVG viewbox that's a bit bigger than a deck slot, and expecting the labware to show up roughly in the middle of it when we plop it in. 2. We're rendering not only a labware, but also a pipette that's supposed to go show up over the labware's wells. So the pipette component needs to be in the same coordinate system as the labware component. I think this was actually buggy before. The labware offset itself by `cornerOffsetFromSlot` (because it had `positioningMode="offsetInSlot"`), but the pipette didn't match that. Therefore: 1. We need `<CenterLabwareInSlot>`, even though we're not rendering a deck slot. 2. We need the `<CenterLabwareInSlot>` to wrap the labware component *and* the pipette component, not just the labware component.
This one is a pretty straightforward labware thumbnail. Formerly, we were making the labware offset itself (`positioningMode="offsetInSlot"`), then manually predicting where it ended up using `getSchema2CornerOffsetFromSlot()`. Now we don't let the labware offset itself (`positioningMode="passThrough"`), and we directly get the thumbnail viewbox with `getLabwareViewBox()`.
This code was already using a `useLayoutEffect()` to automatically compute an SVG viewbox that enclosed the labware, so it never really mattered how the labware was translated in 2D space. The viewbox would automatically translate to follow it, no matter what. So we can just flip `offsetInSlot` to `passThrough` here without needing to change anything else.
SyntaxColoring
force-pushed
the
refactor_offsetinslot
branch
from
d6994ca to
17a098c
Compare
SyntaxColoring
changed the base branch from
fix_labware_rendered_off_center
to
edge
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## edge #20302 +/- ##
==========================================
- Coverage 25.58% 25.58% -0.01%
==========================================
Files 3640 3640
Lines 303120 303129 +9
Branches 42327 42327
==========================================
Hits 77552 77552
- Misses 225544 225553 +9
Partials 24 24
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Continued efforts to migrate away from
positioningMode="offsetInSlot"/"offsetToSlot", where<LabwareRender>and<Module>have some built-in SVG transforms, topositioningMode="passThrough", where they don't. This goes towards EXEC-1278. See the documentation on<LabwareRender>and<Module>for more background onpositioningMode.After this PR, it looks like the only things left that are using
positioningMode="offsetInSlot"/"offsetToSlot"are in protocol-designer.This depends on PR #20285 and will need to merge after it.
Test Plan and Hands on Testing
The
JogToWell/LPCLabwareJogRenderchanges are the ones that worried me the most, and I've checked those in the desktop app.Changelog
Update various call sites of
<LabwareRender>to usepositioningMode="passThrough". This involves understanding what each call site was trying to do and making adjustments to its stack of SVG transforms, if necessary.Review requests
See the individual commit messages for notes on each call site.
Risk assessment
Low risk, given the test plan above, I think. But there's a chance this will cause labware or modules to be rendered in the wrong position.