alphagov / govuk_publishing_components

A gem to document and distribute frontend components for GOV.UK applications
https://components.publishing.service.gov.uk
MIT License
64 stars 20 forks source link

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

Closed owenatgov closed 3 years ago

owenatgov commented 3 years ago

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 commented 3 years ago

🤔 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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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.