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(app,components): fix module controls no module connected case #14784

Merged
merged 4 commits into from
Apr 3, 2024

Conversation

koji
Copy link
Contributor

@koji koji commented Apr 2, 2024

Overview

While I was working on Protocol Run page for RTP, I noticed that Connect modules to see controls is different from No Parameters even they are in the same page.

Mel updated the design.
https://www.figma.com/file/l7BAJAGICr10ELsSqQD0Sr/Design-System%3A-Desktop?node-id=14096%3A7516&mode=dev

Rename NoParameters to InfoScreen to utilize one component for both cases.

https://www.figma.com/file/l7BAJAGICr10ELsSqQD0Sr/Design-System%3A-Desktop?node-id=13146%3A50213&mode=dev#757569773

close AUTH-264

[before]
Screenshot 2024-04-02 at 5 15 13 PM

[after]
Screenshot 2024-04-02 at 5 41 33 PM

Test Plan

  1. make teardown-js && make setup-js
  2. Setup a protocol that requires a module with a robot that doesn't attach any module or remove modules from text-flex.json
  3. Click Module Controls

Changelog

  • Update NoPrameter component and its test
  • Update Module Controls padding

Review requests

If you have any better name for the component, please let me know.

Risk assessment

low

Connect modules to see controls doesn't align with the design. This PR fixes. The component is the
same as NoParameters component so I rename and add a new prop to use only one component.

close AUTH-264
@koji koji requested review from ncdiehl11 and jerader April 2, 2024 21:54
@koji koji marked this pull request as ready for review April 2, 2024 21:55
@koji koji requested a review from a team as a code owner April 2, 2024 21:55
@koji koji requested review from y3rsh and removed request for a team April 2, 2024 21:55
@koji koji requested a review from a team April 3, 2024 06:51
@koji koji removed the request for review from a team April 3, 2024 18:47
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. Question for the future— how do we plan to handle translations for these kinds of components?

@koji
Copy link
Contributor Author

koji commented Apr 3, 2024

This looks good. Question for the future— how do we plan to handle translations for these kinds of components?

Actually, I've been thinking about that.
Ideally just passing text to the component would be good. But for that probably we would need to unify this component for web and odd.

Also, I think solution would depend on UI components' goal since @opentrons/components is published.
https://www.npmjs.com/package/@opentrons/components

@koji koji merged commit 5c3f08b into edge Apr 3, 2024
46 checks passed
@koji koji deleted the fix_protocol-run-no-module-state-styling branch April 3, 2024 22:19
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
…4784)

* fix(app,components): fix module controls no module connected case
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