Use new button styles sitewide #1470
Conversation
(Phy): prefer `solid` and `keyline` over `primary` and `secondary`
Changed width of "log out everywhere" button
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## redesign-2024 #1470 +/- ##
=================================================
- Coverage 40.65% 40.45% -0.21%
=================================================
Files 494 495 +1
Lines 22032 22120 +88
Branches 7302 7333 +31
=================================================
- Hits 8957 8948 -9
- Misses 12450 13135 +685
+ Partials 625 37 -588 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Looking over things, everything looks reasonable on both sites.
That said, it seems a shame to have so many siteSpecifics around if we're planning to replace them so soon anyway. This seems to me like the perfect opportunity to instead get rid of that old language altogether now (i.e. replacing these classes with btn-solid and btn-keyline respectively, and no longer using the outline prop). It should make things way cleaner if we're changing all these lines of code anyway.
It's more work now, but it should be less work overall than changing it all again soon.
| @@ -36,7 +36,7 @@ const GroupJoinPanel = () => { | |||
| e.preventDefault(); | |||
| }}} | |||
| /> | |||
| <Button onClick={processToken} outline color="secondary"> | |||
| <Button onClick={processToken} color="primary"> | |||
There was a problem hiding this comment.
This should be "solid", surely?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
[VRT] Update baselines for redesign/button-styling 0.006% change on "Connect" button
|
OK, I've changed most of the buttons on both sites to use the new classnames. I've left a few There are no intentional visual changes to Ada here. There are a few changes to physics, but hopefully nothing controversial (mostly making sure that cancel/confirm options are always different styles to each other). |
There was a problem hiding this comment.
Nice! I haven't been through and checked every page, but I've had a look through each critical path on both sites and some specific cases where I noticed odd behaviour. It's mostly fine, but there are a couple of odd things to fix (partly from the initial migration months ago which may as well be sorted now). Nothing too major.
I wouldn't mind using some siteSpecifics(keyline, solid)s for instances where there actually IS a difference between the sites (that's what its for!) - but I'm also perfectly happy for them to be dealt with when/if the button is ever changed. We can leave them for now.
(+ I also noticed some spots in the content where this old naming remains, see Trello)
| @@ -37,7 +37,7 @@ export const additionalManagerSelfRemovalModal = (group: AppGroup, user: Registe | |||
| buttons: [ | |||
| <Row key={0}> | |||
| <Col> | |||
| <Button block outline color="primary" onClick={() => { | |||
| <Button block color="keyline" onClick={() => { | |||
There was a problem hiding this comment.
One of these "Cancel"/"Confirm" buttons ought to be solid, and the other keyline. It isn't the case for the current redesign site but is for Ada and the live site (which is how it should be).
Given the nature of confirming a negative effect, I'm not sure I have strong opinions over which way round it should be (solid black to act as a warning for proceeding? or to indicate cancellation? - both are quite standard), as long as they are kept different colours.
There was a problem hiding this comment.
I've gone with solid to cancel, keyline to confirm on physics. I'm not sure it's the best way round (and indeed it's the opposite way to Ada) but it's consistent with most other places in the redesign - we can always swap them all over later.
(I've also swapped some secondarys for siteSpecific(keyline, solid) here to highlight that the two buttons are always opposite styles).
(Phy): prefer
solidandkeylineoverprimaryandsecondaryThis introduces a lot of
siteSpecifics which it would be nice to simplify eventually, but from a user perspective all of the buttons should now use the new styling.