Work out what to do with styled links in the notification banner component

[36degrees]

What

Links in the error summary are red, because we target them in the component CSS:

govuk-frontend/src/govuk/components/error-summary/_index.scss

Lines 39 to 53 in 2b7a162

	.govuk-error-summary__list a {
	@include govuk-typography-weight-bold;
	// Override default link styling to use error colour
	&:link,
	&:visited,
	&:hover,
	&:active {
	color: $govuk-error-colour;
	}
	&:focus {
	@include govuk-focused-text;
	}
	}

We've flagged in #1732 that this seems to break the BEM principles applied elsewhere, where we style things explicitly as elements using classes, rather than using tags in the selector.

From talking to @christopherthomasdesign, when we build the notification banner, we want to make the links red for the error variant and green for the success variant, so we'll have a similar issue.

We should consider:

• that this is a valid use case to bend the BEM principles and using the same approach we use in the error summary
• creating error and success a link element and modifiers, scoped within the component (for example govuk-notification-banner__link govuk-notification-banner__link--error)
• creating error and success link modifiers (for example govuk-link govuk-link--error)

[vanitabarrett]

Option 1: style by element (a)

Follows the same pattern that we use for the error summary component.

.govuk-notification-banner a { 
  // link styles
}

Pros: follows the existing approach we use for error summary; happens for the user magically - they don't need to remember to add a class.

Cons: not following BEM principles; potential duplication of CSS between error summary and notification banner error state.

Option 2: New notification banner class

Add new classes within the notification banner, e.g: govuk-notification-banner__link.

# HTML
<a href="#" class="govuk-notification-banner__link“>Enter a postcode, like AA1 1AA</a>

# CSS
.govuk-notification-banner--success .govuk-notification-banner__link {
  // link styles
}

This could work in 2 ways:

a. Rely on the parent element having a class that reflects the state (error/success/info), e.g: .govuk-notification-banner--success .govuk-notification-banner__link (see above)

b. Have it's own modifiers, e.g: .govuk-notification-banner__link--success

Pros: scoped to just notification banner, so we can easily change this if needed without affecting anything else;

Cons: option b requires a bit more work from the user and potentially opens up the possibility of "mix and match" styles;

Option 3: New govuk-link modifiers

Add modifiers to the existing govuk-link class, e.g: govuk-link--error.

# HTML
<a href="#" class="govuk-link govuk-link—error govuk-notification-banner__link“>Enter a postcode, like AA1 1AA</a>

# CSS
@mixin govuk-link-style-error {
 // link styles
}

.govuk-link—error {
  @include govuk-link-style-error;
}

.govuk-notification-banner__link {
  // any additional link styles, specific to notification banner
}

Pros: adding to something that users are already familiar with; can be used everywhere

Cons: can be used everywhere (??); would potentially need a custom notification banner class on links anyway, e.g: to make them bold, as we wouldn't want that to apply to the global govuk-link--error class.

Option 4: Target govuk-link within notification banner

Style the notification banner links by targeting the govuk-link class. Differs from Option 3 as it's not introducing a new global class/modifier that can be used everywhere.

# HTML
<a href="#" class="govuk-link">Enter a postcode, like AA1 1AA</a>

# CSS
.govuk-notification-banner .govuk-link {
  // link styles  
}

Pros: users are already familiar with adding govuk-link to links - no extra work

Cons: potentially confusing "magic"

[hannalaakso]

Thanks for breaking down the options @vanitabarrett 👍

I think my preference would be to go with option 1 because:

1. It follows what we do on the error summary so there is precedent
2. The styles are applied automatically so they're more likely to be used correctly
3. It would work for the examples we've looked at

We could make these "baked in styles" into a mixin used by notifications and error summary to reduce duplication in scss.

If later on we found that the automatically applied styles were interfering with what some teams were trying to do (ie. some really custom styles) we could consider introducing a modifier class (eg. govuk-notification-banner--no-baked-in-styles or something) which would allow teams not to have these styles applied.

[36degrees]

Thinking mostly about maintainability and 'BEMness', I think I'm leaning towards either option 2b or 3.

Although option 1 follows the precedent set by the error summary, I think it breaks from the conventions of BEM. I think the point around styles being applied automatically making them easier to use is true, but is also true for many other parts of the codebase where we do not currently break from BEM – I can't see any good reason why we should give the notification banner 'special treatment'. I think the tradeoffs between simplicity of use and maintainability were already considered when we adopted BEM.

Option 4 seems to violate BEM's open/closed principle because it modifies the styles of the link just by them being in a different context.

Option 1 has a specificity of 11, and options 2a and 4 have a specificity of 20 which means that for example a govuk-link--extra-special-link modifier (with a specificity of 10) on the link within the banner might not behave as expected.

Options 2b and 3 both have specificities of 10, and so how they interact with another link modifier (and the govuk-link class) will depend on where they appear in the source order. However, the 'conflict' exists between classes on the same element (rather than a conflict between a modifier on an element and a class on that element's parent), and so it seems to work logically.

[edwardhorsford]

External view: I'd probably go with 2 (slight preference to for variant a) or failing that, 3.

The design system is already quite strong on BEM-ey styles, so I think it follows a similar pattern for this to have each element styled.

[vanitabarrett]

@36degrees @hannalaakso Thanks for your comments everyone! It sounds like there’s a preference for either Option 2 or 3.

I think I’m more tempted towards Option 2 (adding a govuk-notification-banner__link class) rather than adding a new modifier to govuk-link, as that feels a bit more risky and not sure if there’s a benefit/use case outside the banner for adding a new govuk-link style (yet)?

I’ve prototyped what Option 2(b) might look like (using the error-summary, but only because it’s similar to the notification banner)

@hannalaakso How does this sound to you, as you previously said you were leaning towards Option 1?

[hannalaakso]

@vanitabarrett Yes I think what the others have said makes a lot of sense about the dangers of different styles of conflicting. I think 2b is probably the best option. 2a sounds okay as well but it could be a little bit too "magic" to make it clear how the styles get applied.

[vanitabarrett]

Going to go with Option 2(b) for now. If, for some reason, that doesn't work out or we think might cause some issues, it's probably worth revisiting Option 2(a), especially given this guidance: http://getbem.com/faq/#block-modifier-affects-elements

[vanitabarrett]

Styling for notification banner will be done as part of #1981

[vanitabarrett]

Changed approach on this while implementing as we noticed a difference in the way we were styling the notification header and text (based on a parent modifier class) vs links (having its own modifier class). We think it makes more sense for things to be consistent, and as this BEM guidance states it is allowed to style a sub-element based on a parent-element modifier. So we've essentially switched from option 2b to 2a