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

Use IconButton for nicer "edit" and "remove revision" icons, and set default appearance for all buttons #665

Merged
merged 4 commits into from
May 31, 2024

Conversation

julienw
Copy link
Contributor

@julienw julienw commented May 30, 2024

It's probably easier to look at commits separately.

  • Commit 1: uses IconButton instead of Button for some of our icons -- namely the "edit" button and the "remove revision" button. This makes these buttons much nicer in the process, especially on hover. Note that this was needed for setting the variant by default in commit 2, that's why I included it in this PR too.

  • Commit 2: set some defaults for all buttons: variant = contained so that they look like button, and disableElevation to remove box shadows. I think that the shared styles do some of "variant = contained" too and we might want to simplify them later, but I didn't want to do that now. (see https://mui.com/material-ui/customization/theme-components/#theme-default-props about this -- we could also add disableRipple if we want to)

  • Commit 3: remove the "variant" property where appropriate now that it's set globally.

  • Commit 4: update test snapshots

To summarize visible changes:

  • removes the box shadows on buttons like the design says
  • makes the "remove revision" icon a bit bigger
  • makes the hover style for both the "edit" icon and "remove revision" icon nicer, and their hit size bigger
  • Some spacings are slightly different too but I think I managed to keep these changes minimal.

I think that the only debatable change is making the variant contained by default. It can still be overriden on a case by case basis by using variant="text" or "outlined".

Please tell me what you think!

Deploy preview
Production version

Before:
image
After:
image

Copy link

netlify bot commented May 30, 2024

Deploy Preview for mozilla-perfcompare ready!

Name Link
🔨 Latest commit 44b7926
🔍 Latest deploy log https://app.netlify.com/sites/mozilla-perfcompare/deploys/66599c1cdd249c00080e0caa
😎 Deploy Preview https://deploy-preview-665--mozilla-perfcompare.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented May 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.18%. Comparing base (ce5919d) to head (8a4753e).

Current head 8a4753e differs from pull request most recent head 44b7926

Please upload reports for the commit 44b7926 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##             beta     #665   +/-   ##
=======================================
  Coverage   97.17%   97.18%           
=======================================
  Files          67       67           
  Lines        1488     1491    +3     
  Branches      256      257    +1     
=======================================
+ Hits         1446     1449    +3     
  Misses         38       38           
  Partials        4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@julienw julienw requested a review from Carla-Moz May 30, 2024 16:04
@julienw julienw changed the title Use IconButton and set default appearance for all buttons Use IconButton for nicer edit and remove reivsion icons, and set default appearance for all buttons May 30, 2024
@julienw julienw changed the title Use IconButton for nicer edit and remove reivsion icons, and set default appearance for all buttons Use IconButton for nicer "edit" and "remove revision" icons, and set default appearance for all buttons May 30, 2024
Copy link
Contributor

@Carla-Moz Carla-Moz left a comment

Choose a reason for hiding this comment

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

Thanks the buttons styles cleanup! Along with removing the box shadows.

@julienw julienw force-pushed the iconbutton-and-consistency branch from 8a4753e to 44b7926 Compare May 31, 2024 09:44
@julienw julienw enabled auto-merge (squash) May 31, 2024 09:45
@julienw julienw merged commit 432e224 into mozilla:beta May 31, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants