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

refactor(app): TextOnlyButton #15668

Merged
merged 1 commit into from
Jul 16, 2024
Merged

refactor(app): TextOnlyButton #15668

merged 1 commit into from
Jul 16, 2024

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Jul 15, 2024

This is a new button that is in the app components. Its existence is a bit tricky.

This is a kind of button that is just text of a particular color with particular hover states - no background color, no border, etc. It's meant to be the lowest-visual-priority interactable it can be.

We have a component that does this, for the ODD only. It's the tertiaryLowLight state of SmallButton, which you can see in the ODD/Atoms/Buttons tree in storybook. This component definitely wasn't and isn't responsive to desktop styles.

Separately, we heavily used buttons of this style in the modern wizards: PipetteWizard, GripperWizard, LPC. These wizards all define their content bodies via GenericWizardTile, whose stories are under App/Molecules/GenericWizardTile, and that component had a completely separate definition of this button style as a styled-component inline in its implementation.

Now, we want to use a button of that style on desktop, but in Error Recovery, which is a flow that doesn't use GenericWizardTile (because it needs the border look&feel of InterventionModal). So the button needs to be defined outside of GenericWizardTile, and also it needs to be responsive. That means we could do one of the following:

  1. Make all the ODD buttons responsive, somehow dealing with the case(s) that have no semantic equivalent on desktop
  2. Make just that one ODD button responsive, but for all of its possible style types, see above
  3. Make just that one ODD button in that one style responsive, but then passing a different style as a prop would result in a weird render
  4. Just make a new dang component

I picked 4. I don't really feel that great about it. Also it's used in ER footer and so that should look correct now.

Testing

  • Do GenericWizardTile stories still look right well, uh, probably not that useful
  • Do the pipette flows still look right
  • Does the go-back button in ER on both desktop and ODD look right

@sfoster1 sfoster1 requested a review from a team as a code owner July 15, 2024 20:47
@sfoster1 sfoster1 requested review from TamarZanzouri and mjhuff and removed request for a team July 15, 2024 20:47
This is a new button that is in the app components. Its existence is a
bit tricky.

This is a kind of button that is just text of a particular color with
particular hover states - no background color, no border, etc. It's
meant to be the lowest-visual-priority interactable it can be.

We have a component that does this, for the ODD only. It's the
tertiaryLowLight state of SmallButton, which you can see in the
ODD/Atoms/Buttons tree in storybook. This component definitely wasn't
and isn't responsive to desktop styles.

Separately, we heavily used buttons of this style in the modern wizards:
PipetteWizard, GripperWizard, LPC. These wizards all define their
content bodies via GenericWizardTile, whose stories are under
App/Molecules/GenericWizardTile, and _that_ component had a completely
separate definition of this button style as a styled-component inline in
its implementation.

Now, we want to use a button of that style on desktop, but in Error
Recovery, which is a flow that doesn't use GenericWizardTile (because it
needs the border look&feel of InterventionModal). So the button needs to be
defined outside of GenericWizardTile, and also it needs to be
responsive. That means we could do one of the following:
1. Make all the ODD buttons responsive, somehow dealing with the case(s)
that have no semantic equivalent on desktop
2. Make just that one ODD button responsive, but for all of its possible
style types, see above
3. Make just that one ODD button in that one style responsive, but then
passing a different style as a prop would result in a weird render
4. Just make a new dang component

I picked 4. I don't really feel that great about it.
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.

Thanks for doing this. I agree that your approach is probably the best option all things considered. It might be worth alerting design that we've componentized this, so they can start using it in Figma?

@sfoster1 sfoster1 merged commit 6a68b88 into edge Jul 16, 2024
30 checks passed
@sfoster1 sfoster1 deleted the error-recovery-text-button branch July 16, 2024 14:06
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.

None yet

2 participants