banner atom's close button should not shrink

[marcelmoreau]

• ensure that the dismiss button will not shrink when more text is added to banner
• align it to top – I did not carry over the messaging icon stuff here, mostly because it looked ok without it, and thinking about it made my head spin with bigger things I might want to address
• bring SVG attributes such as stroke, stroke-linecap, stroke-linejoin, stroke-width to scss
• I also modified the padding of this atom but I think we need to think more about how these should be padded out

[khamer]

Fixing the shrinking, position, and padding all look good. Let's leave the SVG attributes within the SVG. It's just an example inline SVG, none of the svg attributes make sense to add to Boilerplate.

[marcelmoreau]

It's been said in FED meetings that we should bring these attributes to CSS and remove them from the SVG code.

[mbfitz]

Agreed with @marcelmoreau here. We're trying to style SVGs via CSS when we have control over the code, which we do in this case. Makes it easier since you don't have to override any inline styles.

[khamer]

The SVG code and attributes are not part of Boilerplate. They are in the twig file for the example, that's it. It doesn't make sense to move them to banner.scss.

[marcelmoreau]

Why have &__dismiss in the Sass then, with its 1.5rem width and height? If someone is going to use an SVG-based button, we want them to use it without this in there. If it's not part of Boilerplate then I can rip it out.

[mbfitz]

@marcelmoreau @khamer - OK - I think there was some confusion initially...Looks like there are no inline styles tied to this SVG, but there are attributes. I think we're OK keeping the attributes within the SVG code, but anything setting styles we'd want in our CSS. This is probably ok to leave as-is. In the future, we may want to add an SVG icon example atom, though. Could be a good starting point for an icon/svg sprite example.

[marcelmoreau]

they do the same thing, tho...idk, I know we want to encourage people to leverage things like currentColor and putting stuff into our ABEM'd styles. maybe you could finish this one off, @mbfitz if you want

[khamer]

It probably makes sense to change the attribute within the svg to currentColor, but attributes and inline styles work differently, and the attributes here are part of the embedded SVG, whereas the styles for &__dismiss are styling the <button> element. It doesn't really make sense to remove attributes from SVGs (besides sometimes width and height, which I did) in general, but specifically here it definitely doesn't make sense to pull things like stroke-linecap into banner.scss.

Inline styles have higher specificity than CSS targeting classes; attributes have lower specificity. The markup of the SVG here is just taken from tablericons – the banner component should be built so that users can just delete and replace this SVG with whatever they'd like to use.

[khamer]

Closing this pull request - it wasn't finished, and when I checked out this PR this AM, I couldn't even run fractal. I'm guessing it'd need to be updated before we could do more with this; if you do that, free free to reopen.