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(components,-protocol-designer): fix order and border radius logic for DeckLabelSet #17230

Merged
merged 4 commits into from
Jan 14, 2025

Conversation

ncdiehl11
Copy link
Collaborator

@ncdiehl11 ncdiehl11 commented Jan 9, 2025

Overview

This PR updates the order in which elements of a module/adapter/labware stack are displayed. The correct order should be top-down, reflecting the physical orientation of the elements on the deck. Also, this PR fixes the border radii of each label in the set according to designs.

Specifically, the order from top-down should be:

  1. labware
  2. adapter
  3. module

Border radii (all 4px) should be:

  • top element: only bottom right radius
  • middle elements (if they exist): top and bottom right radius
  • bottom element: bottom left and right, and top right radius
Screenshot 2025-01-03 at 1 13 21 PM

Closes RQA-3736

Test Plan and Hands on Testing

  • create or import a protocol with labware/adapter/module stacked
  • select labware and open edit hardware/labware
  • verify style of deck label set according to above

Changelog

  • fix order of elements of deck label set
  • fix radii

Review requests

see test plan

Risk assessment

low

… for DeckLabelSet

This PR updates the order in which elements of a module/adapter/labware stack are displayed. The correct order should be top-down, reflecting the physical orientation of the elements on the deck. Also, this PR fixes the border radii of each label in the set according to designs.

Closes RQA-3736
@ncdiehl11 ncdiehl11 self-assigned this Jan 9, 2025
@ncdiehl11 ncdiehl11 requested review from koji and jerader January 9, 2025 18:39
@ncdiehl11 ncdiehl11 marked this pull request as ready for review January 9, 2025 18:42
@ncdiehl11 ncdiehl11 requested review from a team as code owners January 9, 2025 18:42
@jerader
Copy link
Collaborator

jerader commented Jan 9, 2025

minor bug, seems to only fail with adapters on the deck. works fine with adapters on a mondule

Screenshot 2025-01-09 at 13 51 34

text: def.metadata.displayName,
isSelected: true,
isLast: hoveredLabware == null && selectedNestedLabwareDefUri == null,
const hoverLabelLabel = {
Copy link
Contributor

Choose a reason for hiding this comment

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

hoverLabwareLabel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

typo! i will fix. thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

props in this component aren't optional so I recommend you use === / !== instead of ==/!=

Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

lgtm! don't forget to target the release branch

@ncdiehl11 ncdiehl11 changed the base branch from edge to chore_release-pd-8.3.0 January 14, 2025 17:50
@ncdiehl11 ncdiehl11 merged commit 18f9fb4 into chore_release-pd-8.3.0 Jan 14, 2025
52 checks passed
@ncdiehl11 ncdiehl11 deleted the pd_fix-decklabelset branch January 14, 2025 18:13
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.

3 participants