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(components): add primitive btns and fix useHover on disabled buttons #5972

Merged
merged 4 commits into from
Jun 23, 2020

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Jun 22, 2020

overview

A very common pattern that comes up when using our buttons is: "display a tooltip when this button is disabled". There are two problems that prevent us from doing this easily with out current button components:

  • When disabled, our existing button CSS adds pointer-events: none to the button node, preventing enter and leave events
  • When a <button> is disabled, React will not trigger mouseleave events (nor mouseenter events, starting at React v16.13), even if pointer-events aren't touched and sometimes even if the handlers are attached to the button's parent

This PR tackles both of these issues so that we can easily display tooltips on disabled buttons:

  • Added new buttons in primitives that don't come with any of our existing button baggage:
    • <Btn> - <button> with CSS reset and default type="button"
    • <PrimaryBtn> - <Btn> styled to look like a "Primary Button" from the Zeplin components guide
    • <SecondaryBtn> - <Btn> styled to look like a "Secondary Button" from the Zeplin components guide
    • <LightSecondaryBtn> - <Btn> styled to look like a "Light Secondary Button" from the Zeplin components guide
    • <TertiaryBtn> - <Btn> styled to look like a "Tertiary Button" from the Zeplin components guide
  • Switched the useHover hook from MouseEvents to PointerEvents

changelog

  • feat(components): add primitive btns and fix useHover on disabled buttons
  • reafactor(app): Switch several <OutlineButton> components to <SecondaryBtn> components to demonstrate usage and tooltips

review requests

Components where useHoverTooltip is used and smoke testing is needed

  • app/src/components/CheckCalibration/EndOfStepComparisons.js
  • app/src/components/CheckCalibration/PipetteComparisons.js
  • app/src/components/RobotSettings/CheckCalibrationControl.js
  • protocol-designer/src/components/StepCreationButton.js
  • protocol-designer/src/components/modals/EditModulesModal/index.js
  • protocol-designer/src/components/modules/ModuleRow.js
  • protocol-designer/src/components/StepEditForm/fields/ProfileItemRows.js
  • protocol-designer/src/components/StepEditForm/fields/ChangeTipField/ChangeTip.js
  • protocol-designer/src/components/StepEditForm/fields/TipPositionField/index.js
  • protocol-designer/src/components/StepEditForm/fields/WellOrderField/index.js
  • protocol-designer/src/components/steplist/AspirateDispenseHeader.js
  • protocol-designer/src/components/steplist/MixHeader.js
  • protocol-designer/src/components/steplist/ModuleStepItems.js
  • protocol-designer/src/components/steplist/SubstepRow.js

risk assessment

Button are important and this changes the handlers used for all existing usages of useHoverTooltip, so this is medium risk. The following mitigation steps were taken:

  • Heavily tested the new Btn components down to CSS via jest-styled-components
  • Avoided touching existing <Button> components
  • Only switched out two existing buttons in the app

@mcous mcous added feature Ticket is a feature request / PR introduces a feature app Affects the `app` project ready for review protocol designer Affects the `protocol-designer` project components Affects the `components` project labels Jun 22, 2020
@mcous mcous requested a review from a team as a code owner June 22, 2020 16:57
@mcous mcous requested review from IanLondon, b-cooper, Kadee80 and shlokamin and removed request for a team June 22, 2020 16:57
Copy link
Contributor

@IanLondon IanLondon left a comment

Choose a reason for hiding this comment

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

🆗

  • protocol-designer/src/components/StepCreationButton.js
  • protocol-designer/src/components/modals/EditModulesModal/index.js
  • protocol-designer/src/components/modules/ModuleRow.js
  • protocol-designer/src/components/StepEditForm/fields/ProfileItemRows.js
  • protocol-designer/src/components/StepEditForm/fields/ChangeTipField/ChangeTip.js
  • protocol-designer/src/components/StepEditForm/fields/TipPositionField/index.js
  • protocol-designer/src/components/StepEditForm/fields/WellOrderField/index.js
  • protocol-designer/src/components/steplist/AspirateDispenseHeader.js
  • protocol-designer/src/components/steplist/MixHeader.js
  • protocol-designer/src/components/steplist/ModuleStepItems.js
  • protocol-designer/src/components/steplist/SubstepRow.js

Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

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

  • app/src/components/CheckCalibration/EndOfStepComparisons.js
  • app/src/components/CheckCalibration/PipetteComparisons.js
  • app/src/components/RobotSettings/CheckCalibrationControl.js

@mcous mcous merged commit 57cc219 into edge Jun 23, 2020
@mcous mcous deleted the components_prmitive-btn branch June 23, 2020 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project components Affects the `components` project feature Ticket is a feature request / PR introduces a feature protocol designer Affects the `protocol-designer` project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants