424 default link style #427

JBCSU · 2019-06-10T21:27:50Z

READY FOR REVIEW

Summary

Update the default styling for links. See #424.

Needed By (Date)

Other link work in Decanterpalooza will build on this, so sooner is better than later.

Urgency

Other link work in Decanterpalooza will build on this

Steps to Test

Pull the branch
Rebuild the assets and the styleguide
Review the link styles.

Note that most of the diffs in this PR are because it's based on the 386-nav-color-map branch. I've changed the following files

scss/components/alert/_alert.scss
scss/utilities/functions/color.scss
scss/utilities/mixins/button/_button-secondary.scss
scss/utilities/mixins/link/_link.scss
scss/utilities/placeholders/components/_cta.scss
scss/utilities/variables/core/_core-color-maps.scss
templates/components/link/link.json
kss/builder/decanter/scss/_base_elements.scss

Affected Projects or Products

Create additional link components #341 will depend on this

Associated Issues and/or People

This PR depends on PR Set up color maps for Decanter and refactor main nav colors to refer to color maps #420, as it uses the color maps defined in that PR. That PR should be approved and merged first.
@kerri-augenstein @yvonnetangsu

@JBCSU @yvonnetangsu given that we've effected so many other components (which I imagine may have been one of John's reasons for going with the Regular and non-underlined style to begin with), should we revert back, and add in a "default inline link" style, and let the default "body" style be as previously designed.
I like this because...
It does not force us to undo the Semi-bold and Underline across infinite numbers of links on our sites, which we would inevitably be forced to do. Lots of lines of code we could avoid.
A nice simple default "body" style seems good to me. Regular weight and no underline.

I agree with @kerri-augenstein. Could we revert the default link font-weight and have another variant for link within paragraph or something? :)

I'm not sure how we would apply a "default inline link" style variant. I certainly don't think we want to require users to add a special class to every link they put anywhere in there content. But if they apply the class to a larger container, e.g <body>, <main>, <article>, then our other components would end up inheriting the styling anyway.

More generally, our components are opinionated about how they look - we have a particular look we want to enforce. I think it's appropriate to be explicit about the styling of links in our components. Suppose a user decides they want their links to be bold even if Decanter's default link style doesn't include the font-weight. Then all our components would inherit that anyway.

We only have 14 components, and most of them work ok already. It's only a few components that need special attention. I'll just go ahead and be explicit about the link styling in those components.

That's a good point, @JBCSU

@warn

* 386-nav-color-map: Change @warn to @error in function @color Namespace color map variable Fix styleguide comments Move PR template (#416)

JBCSU · 2019-06-11T19:27:21Z

Ok, I think I've fixed all the cascades. Can someone with a keener eye for fonts than I double check, please? @yvonnetangsu

yvonnetangsu · 2019-06-11T19:45:18Z

Ok, I think I've fixed all the cascades. Can someone with a keener eye for fonts than I double check, please? @yvonnetangsu

Font weight looks good, @JBCSU , but what happens to the underline? I thought we're adding underline to the default link style but I could be wrong 🤔 . Also, I believe the links in alerts have underlines to begin with.

yvonnetangsu · 2019-06-11T19:46:44Z

One more thing, the :focus state of the default link needs to be changed to black color also (should match the :hover state when keyboard user tab to it). And the animation - those apply to the :focus as well.

JBCSU · 2019-06-11T20:38:29Z

Ok, I think I've fixed all the cascades. Can someone with a keener eye for fonts than I double check, please? @yvonnetangsu

Font weight looks good, @JBCSU , but what happens to the underline? I thought we're adding underline to the default link style but I could be wrong 🤔 . Also, I believe the links in alerts have underlines to begin with.

The new default link style does have underline. I therefore removed the specification from links inline in alert text and allow those links to inherit the default style, as I believe those links should actually get the default inline link style (i.e. if a developer overrides the default inline styles).

JBCSU · 2019-06-11T20:40:46Z

One more thing, the :focus state of the default link needs to be changed to black color also (should match the :hover state when keyboard user tab to it). And the animation - those apply to the :focus as well.

:focus was not in the css before, but I can add it.

Animation is not part of the default inline link style, which is this PR. If we need to address animation, I propose we do it in #341.

yvonnetangsu · 2019-06-11T20:44:29Z

Ok, I think I've fixed all the cascades. Can someone with a keener eye for fonts than I double check, please? @yvonnetangsu

Font weight looks good, @JBCSU , but what happens to the underline? I thought we're adding underline to the default link style but I could be wrong 🤔 . Also, I believe the links in alerts have underlines to begin with.

The new default link style does have underline. I therefore removed the specification from links inline in alert text and allow those links to inherit the default style, as I believe those links should actually get the default inline link style (i.e. if a developer overrides the default inline styles).

Great - yeah, turns out I had to rebuild the styleguide. Looks good to me.

yvonnetangsu · 2019-06-11T20:46:05Z

One more thing, the :focus state of the default link needs to be changed to black color also (should match the :hover state when keyboard user tab to it). And the animation - those apply to the :focus as well.

:focus was not in the css before, but I can add it.

Animation is not part of the default inline link style, which is this PR. If we need to address animation, I propose we do it in #341.

Interesting....I didn't realize the current live site doesn't have focus styles for the links. I must be thinking about the homesite (the jump links have animation when you tab to them). We could merge this in, and I can deal with the focus state in the animation branch later?

yvonnetangsu · 2019-06-11T20:50:10Z

One probably final thing, JB - the "Decanter" logo now has underline and the font weight is different. Could you please remove the underline and change it to font-weight: 700?

move all alert colors into the $alert-colors map; change button-big to use button-primary and just make overrides

JBCSU · 2019-06-11T21:43:14Z

I did go ahead and :focus states, and fixed all the cascades' :focus states. And I fixed the link in the masthead. Anything else, @yvonnetangsu? Thanks for your very careful eye!

kss/builder/decanter/kss-assets/css/kss.css

Merge branch 'master' into 424-inline-link-style * master: Set up color maps for Decanter and refactor main nav colors to refer to color maps (#420) # Conflicts: - all resolved using "mine" # core/dist/css/decanter.css # core/src/scss/components/global-footer/_global-footer.scss # core/src/scss/utilities/functions/_color.scss # core/src/scss/utilities/mixins/button/_button-big.scss # core/src/scss/utilities/mixins/button/_button-primary.scss # core/src/scss/utilities/mixins/button/_button-secondary.scss # core/src/scss/utilities/mixins/link/_link.scss # core/src/scss/utilities/variables/core/_core-color-maps.scss

yvonnetangsu

I suggest we change the .su-alert--warning link color to $color-dark-red or $color-cardinal-red, and hover link color to $color-black. The blue is more similar in value (brightness) to the yellow background compared to red. So changing this to red would increase the contrast.

yvonnetangsu · 2019-06-11T22:26:28Z

GTG for me! The Decanter logo hover color - I can live with either.

update doc for alert color map in styleguide

yvonnetangsu

Looks great - thanks for updating the color map and adding the focus states! 👍

sherakama

Thanks for all your attention on this detailed item. GTG.

JBCSU · 2019-06-11T22:43:31Z

Great, thanks @yvonnetangsu & @sherakama. Merging now.

* master: 424 default link style (#427) Update package.json (#438) Set up color maps for Decanter and refactor main nav colors to refer to color maps (#420)

* master: Use svg icons as "mask" so their colors could be changed using CSS background-color (#426) 424 default link style (#427) Update package.json (#438)

yvonnetangsu and others added 29 commits May 28, 2019 14:53

WIP color maps with function to retrieve color; refactor brand bar an…

406c15d

…d alerts to use color map and function; start refactoring main nav colors

WIP refactor main nav colors

b7da694

WIP refactor main nav colors

2d08215

Add warnings to color function

39d28eb

finish up main nav default style color refactor

c2466b8

Finish up refactoring main-nav--dark colors

d8c7295

Fix main-nav--light variant, when mobile search is present the backgr…

8b25375

…ound color is now white; beginning color map refactoring

WIP main-nav--light color refactor

9d3de5a

WIP main-nav--light refactoring; change link-border to border

4f9cd86

Finish refactoring main-nav--light colors to use color map

38f290e

Refactor to replace .su-main-nav.su-main-nav--dark selectors with jus…

7ea298e

…t .su-main-nav--dark

fixup su-main-nav--dark

6f7bc0d

Continue cleanup of main nav styles

6b3c15f

Main-nav code cleanup remove redundancy

f079dcb

WIP fixup

eb04eb2

bg color transition fixup

8d28b55

main-nav--light mobile minus sign fixup

a07c704

have colors in brand bar, global footer, logo, button primary to refe…

597cea0

…r to color map

main-nav--light fixup

aebbeb3

main-nav--light fixup

6df7525

doc, styleguide and button/link colors

a0531d4

fixup

eeb056e

Doc

0e01215

cc indentation fix

ee9e703

fixup

4037b52

Change all reference to $color-dark-red in $main-nav-colors to $color…

23dd066

…-bright-red

change style of inline (default) link style

1d164c2

update doc and sample text

2d0eaf3

Remove nesting of main nav color maps and $theme in function @color

62fe57d

JBCSU added 2 commits June 11, 2019 12:25

fix cascade of <a>'s font-weight into our other components

6b7f0e3

Merge branch '386-nav-color-map' into 424-inline-link-style

ef81fed

* 386-nav-color-map: Change @warn to @error in function @color Namespace color map variable Fix styleguide comments Move PR template (#416)

JBCSU added 2 commits June 11, 2019 14:24

make :focus match :hover, and fix cascades;

af6ee1c

move all alert colors into the $alert-colors map; change button-big to use button-primary and just make overrides

fix logo link styling in styleguide's masthead

b6bd5e3

yvonnetangsu reviewed Jun 11, 2019

View reviewed changes

kss/builder/decanter/kss-assets/css/kss.css Outdated Show resolved Hide resolved

yvonnetangsu reviewed Jun 11, 2019

View reviewed changes

change color of inline links in warning alerts

5069e92

JBCSU added 2 commits June 11, 2019 15:26

use cardinal-red for warning alert links;

54089ae

update doc for alert color map in styleguide

remove hover / focus color from style guide's masthead logo

2c72777

yvonnetangsu approved these changes Jun 11, 2019

View reviewed changes

sherakama approved these changes Jun 11, 2019

View reviewed changes

JBCSU merged commit 80133c4 into master Jun 11, 2019

JBCSU deleted the 424-inline-link-style branch June 11, 2019 22:45

yvonnetangsu added a commit that referenced this pull request Jun 12, 2019

Merge branch 'master' into 423-svg-icon-test

328db3a

* master: 424 default link style (#427) Update package.json (#438) Set up color maps for Decanter and refactor main nav colors to refer to color maps (#420)

yvonnetangsu added a commit that referenced this pull request Jun 12, 2019

Merge branch 'master' into 222-masthead

c353fcd

* master: Use svg icons as "mask" so their colors could be changed using CSS background-color (#426) 424 default link style (#427) Update package.json (#438)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

424 default link style #427

424 default link style #427

JBCSU commented Jun 10, 2019 •

edited

Loading

yvonnetangsu commented Jun 11, 2019

JBCSU commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019 •

edited

Loading

JBCSU commented Jun 11, 2019

JBCSU commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019

JBCSU commented Jun 11, 2019

yvonnetangsu left a comment •

edited

Loading

yvonnetangsu commented Jun 11, 2019

yvonnetangsu left a comment

sherakama left a comment •

edited

Loading

JBCSU commented Jun 11, 2019

424 default link style #427

424 default link style #427

Conversation

JBCSU commented Jun 10, 2019 • edited Loading

READY FOR REVIEW

Summary

Needed By (Date)

Urgency

Steps to Test

Affected Projects or Products

Associated Issues and/or People

See Also

yvonnetangsu commented Jun 11, 2019

JBCSU commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019 • edited Loading

JBCSU commented Jun 11, 2019

JBCSU commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019

yvonnetangsu commented Jun 11, 2019

JBCSU commented Jun 11, 2019

yvonnetangsu left a comment • edited Loading

Choose a reason for hiding this comment

yvonnetangsu commented Jun 11, 2019

yvonnetangsu left a comment

Choose a reason for hiding this comment

sherakama left a comment • edited Loading

Choose a reason for hiding this comment

JBCSU commented Jun 11, 2019

JBCSU commented Jun 10, 2019 •

edited

Loading

yvonnetangsu commented Jun 11, 2019 •

edited

Loading

yvonnetangsu left a comment •

edited

Loading

sherakama left a comment •

edited

Loading