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

Make sure that success alert component is using the correct link styles #2008

Closed
owenatgov opened this issue Apr 7, 2021 · 4 comments
Closed

Comments

@owenatgov
Copy link
Contributor

The new success alert component, which is inherited from the success variant of the design system notification banner, uses an explicit class to style it's links to be green (govuk-notification-banner__link).

Some comments from the design system team on the link colour choice:

I believe it was a design/consistency thing, so the link looked consistent within its container. It's also similar to what we do for the error summary component.

Yeah it’s to make each kind of banner look like its own distinct thing, and have a more cohesive appearance. It looks a bit wrong, even broken, when the links are a different colour to the banner! I’d worry about GOV.UK veering from the standard component on something like this just cos service teams might copy it

Further reading on the implementation choice

This can't be implemented at the component level as we don't have total control over what content will be coming in, so we need to go through the instances of the success alert across govuk apps and make sure that any instances of links within the success alert component are using the above class.

Additionally, guidance should be added to the component docs advising to use the link class if you success alert content includes a link.

@chris-gds
Copy link
Contributor

🤔 Would something like this work? I'm trying to think, ideally you want to inherit, apply to all, but also scope

.gem-c-success-alert {
  a {
    @extend .govuk-notification-banner__link;
  }
}

@owenatgov
Copy link
Contributor Author

owenatgov commented Apr 7, 2021

@chris-gds This could work yes, but the DS team have made an informed decision about the specific implementation and I'd rather lean on said implementation than try to roll our own. Check out the convo in the DS issue I linked to for some more detailed whys.

@owenatgov
Copy link
Contributor Author

I've had a nosey through the uses of success alert and haven't found any that use links in their body besides the cookie banner. I'm going to make a PR with some extra guidance on the success alert in a bit.

@owenatgov
Copy link
Contributor Author

Marking as resolved for now. It doesn't look like there's a great need to use links in success alert components across govuk at present. I may bring this up in a future frontenders catchup to gauge thoughts on this.

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

No branches or pull requests

2 participants