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) modify the drag preview position #17576

Merged
merged 8 commits into from
Mar 6, 2025

Conversation

koji
Copy link
Contributor

@koji koji commented Feb 24, 2025

Overview

[before]

drag_and_drop_before.mov

[after]

drag_and_drop_after.mov

At this moment, the design team and the product team approved this update.
I think further improvement would need the design team's input.

close AUTH-1469

Test Plan and Hands on Testing

Changelog

Review requests

Risk assessment

low

@koji koji changed the base branch from edge to chore_release-pd-8.4.1 February 24, 2025 15:38
@koji koji requested a review from ncdiehl11 February 24, 2025 15:48
@koji koji marked this pull request as ready for review February 24, 2025 15:48
@koji koji requested a review from a team as a code owner February 24, 2025 15:48
@ncdiehl11
Copy link
Collaborator

It seems a bit buggy. You can't select the top part of the labware to drag. I wonder if react dnd provides an API to move the svg relative to where the cursor is positioned? I will do some digging

@koji koji requested review from syao1226 and removed request for a team February 24, 2025 17:54
@koji koji changed the base branch from chore_release-pd-8.4.1 to chore_release-pd-8.4.3 February 25, 2025 23:07
@koji koji requested a review from jerader March 4, 2025 18:58
Copy link
Collaborator

@ncdiehl11 ncdiehl11 left a comment

Choose a reason for hiding this comment

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

This looks good! one nit but not blocking

@@ -9,51 +9,97 @@ import {
} from '@opentrons/components'
import { DND_TYPES } from '../../../../constants'
import { DECK_CONTROLS_STYLE } from '../constants'
import previewImage from '../../../../assets/images/preview.svg'
Copy link
Collaborator

Choose a reason for hiding this comment

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

can move up

Copy link

codecov bot commented Mar 6, 2025

Codecov Report

Attention: Patch coverage is 3.77358% with 51 lines in your changes missing coverage. Please review.

Please upload report for BASE (chore_release-pd-8.4.3@f1bb180). Learn more about missing BASE report.

Files with missing lines Patch % Lines
.../pages/Designer/DeckSetup/Overlays/DragPreview.tsx 3.77% 51 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                    Coverage Diff                    @@
##             chore_release-pd-8.4.3   #17576   +/-   ##
=========================================================
  Coverage                          ?   17.79%           
=========================================================
  Files                             ?     3075           
  Lines                             ?   221782           
  Branches                          ?     6866           
=========================================================
  Hits                              ?    39467           
  Misses                            ?   182315           
  Partials                          ?        0           
Flag Coverage Δ
protocol-designer 17.79% <3.77%> (?)

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

Files with missing lines Coverage Δ
.../pages/Designer/DeckSetup/Overlays/DragPreview.tsx 4.10% <3.77%> (ø)
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@koji koji merged commit a68d111 into chore_release-pd-8.4.3 Mar 6, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants