-
Notifications
You must be signed in to change notification settings - Fork 107
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
Update secondary button style #1077
base: main
Are you sure you want to change the base?
Conversation
the states make sense and seem logical to me |
Fixed the BackstopJS tests. It looks like support is being removed for disabled buttons #1075 - I wonder if this needs updating here too? |
We're thinking of removing the disabled class, which we think is only used for links styled as buttons, leaving the |
This is looking good to me @davidhunter08! Nice work. Bottom radius stroke is ever so slightly funky but I can't think of a way to fix it, I'm sure you've thought about it too. Only think I can notice is the pseudo |
Thanks @anandamaryon1. Good spot on the overlay overhang, I've fixed that. Re: Bottom radius stroke, yeah it's a bit annoying, I've tried a few implementations but that's the best I've got so far. I'll try some other things this afternoon. |
@davidhunter08 thanks, option 2 looks to solve it, but needs focus style tweaks too:
|
Thanks @anandamaryon1, I've updated the code to option 2 and sorted the focus state. |
Looks good to me! I’ve been avoiding using the existing secondary button in our service as some users in research sessions have assumed it was inactive/disabled, and this should solve that. |
@@ -7,12 +7,18 @@ | |||
* 2. Fix unwanted button padding in Firefox. | |||
* 3. Use a pseudo element to expand the click target area to include the | |||
* button's shadow as well, in case users try to click it. | |||
* 4. Makes the secondary button that has a border the same height as the other | |||
* buttons that don't have a border. | |||
*/ | |||
|
|||
// Because the shadow (s0) is visually 'part of' the button, we need to reduce | |||
// the height of the button to compensate by adjusting its padding (s1) and | |||
// increase the bottom margin to include it (s1). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second (s1) should be (s2)?
Just to add my two-penneth.... Changes all look good and make complete sense to me. To tie any changes up in a nice shiny bow though (🎁) it would be good to attach any user research insights to this - just to confirm the original user need, and where the suggested change meets this! 🙂 |
Thanks @StephenHill-NHSBSA - totally agree. I'll link to the research findings once they've been documented in GitHub, which should be this week 👍 |
From a mostly subjective perspective, but also slightly informed by the occasional feedback (I think?) that the current secondary button style sometimes gets mistaken for being a disabled state, I'm surprised to see the secondary button style presented as a 'border only' style – especially one with a transparent background. I recognise that the 'solid button with brand colour bg + bordered button with brand colour border' is a widely-used style for 'Sign-in or Register' adjacent buttons, so perhaps that has now established it as a solid pattern, but I think the use of the same 'action' colour is surely important in those contexts? (All of which is purely visual I appreciate). Given that the primary button colour is solid green, I'd have thought the obvious interpretation of that in the bordered style would be a green border? e.g. DO THIS PRIMARY GREEN THING! Or consider doing this other secondary green thing! Maybe all of this has already been considered – apologies for jumping in a bit cold from a thread elsewhere – but I'd also be intrigued to see the research findings, and whether the green bordered style was considered. Update: Now with a hastily hacked together example via browser hacking: I don't think the above is a perfect design by any means. And I have obviously not made any of the undoubtedly thorough considerations in the proposed solution. But, I think a frequent outcome of the proposed solution, assuming primary and secondary buttons are often used adjacently, would be as below, which doesn't feel like a cohesive design (IMO): Last thing, I understand the preference to do one thing per page. However, in practice, even when supporting one primary thing on a page, there is frequently a need to use two buttons (even allowing for the 'primary button' with cancel as a link pattern). |
Hey @benchilds - yeah we found the same problems on the NHS App, users mistaking the the current (grey) secondary button as disabled. Based on that, and some earlier thinking in the community backlog, we created a design hypothesis around changing the style/colours, which tested well with users. Border styleIn regards to the style, I wouldn't say the design is 'border only' as it has the thicker bottom border that elevates it and gives it affordance, like the primary button. It is similar to the feedback buttons on the GOV.UK: ColoursGreen was considered early on but was deemed not as versatile as the blue (@Tosin-Balogun might be able to expand on this?). On the app there's no instances of the primary and secondary button being used together, but there are instances of the primary and secondary buttons being used with text links. So having it blue makes it feel more cohesive on the app but I do agree that the green is more cohesive when used with a primary button. 111 onlineThe button design and research from 111 online:. |
I don’t have any views on the green vs blue. @davidhunter08 did you explore using a white background colour instead of transparent? That’d have increased text contrast, but possibly it’d stand out too much? |
just my hunch, but an outline (green or blue) with a white BG feels very much like a different style for a primary button if you don't have it right next to the solid green button |
@frankieroberto we considered a white background but we felt a transparent background would be more versatile, as it would work on both grey (page colour) and white (card) backgrounds. All our testing so far has been a transparent background sitting on the grey page colour. |
Good point on considering the colour @benchilds, to add another point to factor in: we're considering adding an NHS login button, which will be essentially a blue primary button. This makes me verge more towards it being green (or grey as it currently is?), and not blue. |
I initially started this issue after some experiences I had trying to design dashboards for clinical systems on screening. Quite a lot of thinking has gone into this already which dates back to 2022. The green idea was considered early on in my sketches but it didn't work well or felt versatile enough to sit within our wider component library. Visually, it also didn't make sense when I tried it within a lot of other components (Tables, Expanders, Cards etc), so I actually abandoned green very early in my thinking. At the time, I was also doing an audit of our component library for other things not just buttons Other stronger candidate was grey but it also either didn't feel versatile enough to be a secondary option and be visible enough at the right amount. We also use blue quite a lot more for call to actions around most of our components, so hierarchy wise, it make sense to start from bottom-up, therefore green exist as a primary loud button. What @benchilds is talking about only works if we had designed the whole system around the green button and did it top-down, but we did not do that. Our visual style starts from neutrals to blue to green (which we use sparingly) Another thing that factor into my thinking was that 111online had been user testing it at the time and I introduced it to campaigns which we tested as well (It tested well with users). We also used to have a 'menu' option on the header prior which used similar style (later replaced with 'more' dropdown). In total, we have tested it on 3 different kinds of products (campaigns, 111online and on the app) @frankieroberto A solid fill white button is literally the reverse primary (i.e the green button but reversed for darker backgrounds), which also removes the hierarchy level it was meant to sit within (as you hinted). PS: This is not a call to change the green primary button to blue because we do not need to become monochromatic for uniformity. The green is our loudest cta and its effective as it is, perhaps that is a different convo |
This was part of the point of making the background transparent, because it inherits whatever background it might sit on which increase its versatility. At the time, I was struggling a lot on screening to find the right level for different parts of the dashboard which was what made me create the original issue. |
@Tosin-Balogun perhaps it needs to be configurable? to set the background to solid or transparent? In the example I shared the secondary couldn't be used in the filter area with grey background, etc. |
@edwardhorsford – I suppose there could be scope for customisation within your service. There should be room for service specific solutions Our original backgrounds do not use that shade of grey, so it didn't factor into the thinking. On that grey background, our hyperlink won't work there either as the grey is more darker than ones we use for backgrounds. Bit history here, our backgrounds used to be white originally, but we moved to the pale grey (which is a desaturated nhs blue actually) after lots of research we did to help reduce screen glare Dean has an article about it somewhere on medium. |
Thanks for the open discussion @davidhunter08 @frankieroberto @edwardhorsford (and everyone else I've probably missed!) and explanation of the design approach @Tosin-Balogun. (Sorry for the delay replying... apparently I've not got notifications enabled for this repo).
|
Description
A proposal to change the secondary button visual style.
Current design
Proposed design
Supporting information
Checklist