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

feat(protocol-designer): update React DnD from version 6.0.0 to 16.0.1 #14485

Merged
merged 17 commits into from
Feb 20, 2024

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Feb 13, 2024

close RAUT-964

Overview

This PR updates React-DnD from version 6 to 16.0.1 and it updates the react dnd backend from mouse-backend to HTML5-backend (this backend has been updated recently, since mouse-backend was last update 5 years ago 💀 )

NOTE: i didn't add test coverage in this because once the vite migration is done, we will be using vitest - figured that is a better time to add test coverage

Test Plan

Test the drag and drop features which include:

  1. dragging/dropping labware to different slots on the deck, check dragging/dropping on top of modules, adapters and on the deck, and swapping labware
  2. dragging/dropping to reorder the step items. The easiest way to do this is import a protocol fixture and drag/drop the items to rearrange the steps. Try this with doItAllV8 or something! Also test deleting & duplicating a step -> which you can access through right clicking on the step item and seeing the "delete" option in the menu that appears.

Please call out anything strange you might see. I tried to smoke test all the options as thoroughly as possible.

Changelog

  • update react DnD and backend version in package.json
  • rename robotCoordinateWorkSpace component to something that includes the ref since it no longer needs the DOM coords
  • update each component to using the react dnd hooks
  • remove all now unnecessary @ts-expect-errors due to the react dnd version

Review requests

see test plan

Risk assessment

low

Copy link

codecov bot commented Feb 13, 2024

Codecov Report

Attention: 160 lines in your changes are missing coverage. Please review.

Comparison is base (c142da1) 67.79% compared to head (e300361) 67.77%.
Report is 51 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #14485      +/-   ##
==========================================
- Coverage   67.79%   67.77%   -0.03%     
==========================================
  Files        2518     2518              
  Lines       72017    72145     +128     
  Branches     9244     9283      +39     
==========================================
+ Hits        48826    48894      +68     
- Misses      20989    21033      +44     
- Partials     2202     2218      +16     
Flag Coverage Δ
app 64.64% <ø> (+0.05%) ⬆️
components 49.04% <ø> (-0.37%) ⬇️
labware-library 41.11% <ø> (ø)
protocol-designer 37.84% <0.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...botCoordinateSpace/RobotCoordinateSpaceWithRef.tsx 0.00% <ø> (ø)
...esigner/src/components/DeckSetup/LabwareOnDeck.tsx 0.00% <ø> (ø)
...ents/DeckSetup/LabwareOverlays/LabwareControls.tsx 0.00% <ø> (ø)
...otocol-designer/src/components/DeckSetup/index.tsx 0.00% <ø> (ø)
...ocol-designer/src/components/steplist/StepList.tsx 0.00% <ø> (ø)
...l-designer/src/components/steplist/ContextMenu.tsx 0.00% <0.00%> (ø)
...ocol-designer/src/containers/ConnectedStepItem.tsx 0.00% <0.00%> (ø)
...rotocol-designer/src/components/ProtocolEditor.tsx 0.00% <0.00%> (ø)
...ponents/DeckSetup/LabwareOverlays/SlotControls.tsx 0.00% <0.00%> (ø)
...ents/DeckSetup/LabwareOverlays/AdapterControls.tsx 0.00% <0.00%> (ø)
... and 2 more

... and 55 files with indirect coverage changes

@jerader jerader force-pushed the pd_update-react-dnd branch from 0c2cd3a to f7a40fb Compare February 13, 2024 18:05
Comment on lines +91 to +92
if (newSlot != null) {
dispatch(moveDeckItem(newSlot, labwareId))
Copy link
Collaborator Author

@jerader jerader Feb 14, 2024

Choose a reason for hiding this comment

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

the reason why i am checking for newSlot here (also in SlotControls) is because I can't get item from useDrag to update to the latest state. If you drag and drop the labware initially, it doesn't update to the new slot in item - this was causing a bug where you could only drag and drop the labware once. I tried to find documentation about how to fix this but i couldn't find anything so this was my workaround. Any refactor suggestions are welcome!

@jerader jerader changed the title feat(protocol-designer): update React DnD to version 16.0.1 feat(protocol-designer): update React DnD from version 6.0.0 to 16.0.1 Feb 14, 2024
@jerader jerader marked this pull request as ready for review February 14, 2024 21:45
@jerader jerader requested a review from a team February 14, 2024 21:45
@jerader jerader requested a review from a team as a code owner February 14, 2024 21:45
@jerader jerader marked this pull request as draft February 15, 2024 18:46
@jerader jerader removed request for a team February 15, 2024 18:46
@@ -486,7 +478,6 @@ export const DeckSetupContents = (props: ContentsProps): JSX.Element => {
</React.Fragment>
)
})}
<DragPreview getRobotCoordsFromDOMCoords={getRobotCoordsFromDOMCoords} />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When i updated from mouse backend to html5 backend, the DOM coordinates changed and resulted in the drag preview being mirrored. I tried to fix it but couldn't quite get it right because the EditLabware was being shown in replace of the cursor and it was being layered on top of the drag preview that was not in the correct place. so I opted to delete it. I showed the current visuals to Felix and he said they are ok! the deck map is getting a whole redesign this year (for 8.2) so this change will be only for the upcoming PD release (8.1).

@jerader jerader marked this pull request as ready for review February 16, 2024 18:35
@jerader jerader requested a review from a team as a code owner February 16, 2024 18:35
@jerader jerader removed the request for review from a team February 16, 2024 18:35
@jerader jerader requested review from b-cooper and a team February 16, 2024 18:35
Copy link
Contributor

@mjhuff mjhuff left a comment

Choose a reason for hiding this comment

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

Smoke tested without any issues or strange behavior!

Bummer about the item from useDrag not updating, but the newSlot approach is easy enough to follow. LGTM! Thanks for spending the time to update 10 versions 🥳

@jerader jerader merged commit aeef380 into edge Feb 20, 2024
31 of 32 checks passed
@jerader jerader deleted the pd_update-react-dnd branch February 20, 2024 21:29
@jerader jerader mentioned this pull request Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants