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

Add NHS login buttons (WIP for discussion) #992

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

edwardhorsford
Copy link
Contributor

@edwardhorsford edwardhorsford commented Aug 2, 2024

Fixes nhsuk/nhsuk-service-manual-community-backlog#374

Description

This adds the NHS login and login reverse button variants to the design system. It's not an exact recreation, so for discussion. I suspect where the existing login button styles differ from the design system may not be intentional, so we should consider making their active and focus styles align with other design system buttons. If we can track down the source SCSS, it might also be helpful to understand how they were made.

The existing design system button is repetitive and could do with a refactor, but for now I've just created them in a similar way to the existing alternate button styles.

This doesn't cover inserting the NHS logo within the button - perhaps it should?

Differences:

  • Shadow blue is very subtly different
  • Design system buttons have different hover and active colours, whereas the NHS login one didn't
  • When focused the original has dark blue text and shadow - whereas the rest of the design system goes to black
  • Active styles - the design system reverse button goes to black when active - whereas the original NHS login one stayed white. I'm not sure which I prefer to these, but default to following how the reverse button style works here for consistency.

Regular login button:

Original New
Screenshot 2024-08-02 at 09 57 20 Screenshot 2024-08-02 at 09 59 50
Focused Focused
Screenshot 2024-08-02 at 10 00 24 Screenshot 2024-08-02 at 10 00 45
Active Active
Screenshot 2024-08-02 at 10 02 03 Screenshot 2024-08-02 at 10 02 10
Video Video
nhs-login-original nhs-login-new

Reverse login button

Original New
Screenshot 2024-08-02 at 10 10 27 Screenshot 2024-08-02 at 10 10 44
Focused Focused
Screenshot 2024-08-02 at 10 11 19 Screenshot 2024-08-02 at 10 11 32
Active Active
Screenshot 2024-08-02 at 10 12 34 Screenshot 2024-08-02 at 10 12 40 (this matches regular reverse style)
Video Video
nhs-login-reverse-old nhs-login-reverse-new

@edwardhorsford edwardhorsford changed the title Add NHS login buttons Add NHS login buttons (WIP for discussion) Aug 2, 2024
@paulrobertlloyd
Copy link
Contributor

paulrobertlloyd commented Aug 2, 2024

Quick thoughts:

  • Is there enough difference between the reversed button and reversed login button for these to be defined separately? Personally, I would take the styles you’ve defined here for the reversed login button and apply them to the existing reversed button.
  • As to the logo, I believe the guidance is that only one NHS logo should appear on a given page. Therefore, if components in the NHS.UK Design System are intended to go on NHS-branded websites, maybe the logo isn’t needed (and adding it would increase the likelihood of it being added when it shouldn’t). Maybe guidance can explain how the NHS logo could be added, instead.

Also worth nothing that, in any guidance associated with this new button variant, and with any co-ordination needed with IAM teams within NHS England, there is also the Care Identity service, which provides login management for NHS staff. Their guidance is similar to that for NHS Login.

@roshaanbajwa roshaanbajwa self-requested a review August 5, 2024 11:14
@anandamaryon1
Copy link
Contributor

Good points @paulrobertlloyd

  • Difference from reverse button: agree that they are very similar. I wonder whether we could get away with not adding the login button reverse, to reduce the number of button variants we have? I don't think we should change the standard reverse button to have blue text though.
  • NHS logo: I was imagining we didn't need the logo version, as you mention, our users tend to be NHS branded already. However, for Care Identity perhaps it'd be useful to include the icon as an option, so that we don't have slightly different implementations of the logo in the button. Maybe we don't show it as an on-screen example in the design system but we do mention it in guidance and possibly also include it as a nunjucks option?

@frankieroberto
Copy link
Contributor

However, for Care Identity perhaps it'd be useful to include the icon as an option, so that we don't have slightly different implementations of the logo in the button.

The Care Identity button guidance suggests always having a logo, but that does contract the NHS brand guidance of only having a single logo per page.

I wonder if we should avoid having the logo in the button for now - for both NHS login and Care Identity, and instead seek to update the Care Identity button guidance for NHS branded staff-facing services?

@paulrobertlloyd
Copy link
Contributor

paulrobertlloyd commented Aug 20, 2024

Assuming the button can accept HTML, that provides a way to include the logo, so not including a logo option in the component, at least to start off with, seems fine to me.

@chrimesdev
Copy link
Member

LGTM, as discussed it would be good to try include the NHS logo to see if the current styles work or if there is more required

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NHS login button
6 participants