Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter #504

yvonnetangsu · 2019-10-02T00:52:14Z

READY FOR REVIEW

Summary

Add a parameter $vertical in the @link-icon mixin so users can adjust the vertical position of the icon by adding a corresponding margin-bottom to the ::after pseudo element of the link. Positive value moves icon up; negative moves it down.
Add options for $animate: right, up, down, topright (diagonally towards upper right corner).
Finetune icon vertical positions of existing link component variants.

Needed By (Date)

Whenever

Urgency

Not urgent

Steps to Test

Pull this branch and compile styleguide.
Go to the compiled styleguide, look at Components -> Links, check that all the icons of existing link variants have good vertical alignments with the link text and that they animate as specified in Tweak Section 3.9 - Links #505
Open a link variant scss file, e.g., components/link/_link--internal.scss, change the $vertical and $animate parameters and check that they behave as expected.

Affected Projects or Products

Decanter and any sites that use link variants that uses SVG masks

Associated Issues and/or People

… tune existing link variants icon vertical positions

sherakama · 2019-10-02T23:24:35Z

core/src/scss/utilities/mixins/link/_link-icon.scss

 // $animate {boolean} - Whether or not to animate on hover.
 //
 // Style guide: Mixins.Link.link-icon
 //
-@mixin link-icon($icon, $width: 0.65em, $animate: true) {
+@mixin link-icon($icon, $width: 0.65em, $vertical: null, $animate: true) {


Technically, changing the order of the parameter input is changing API so in order to not break API, you will need to add it to the end of the mixin parameters. It is much cleaner where it is right now, so if you would like to put this in as a PR to v6 I'll be ok with where it is.

Also, shouldn't the default value for the vertical alignment be -1px as that was the original value?

Sounds good, @sherakama, and thanks for reviewing. When is v6 coming out? Is it after Decanterpalooza 3 this Dec? If so, then it's totally fine that we put this in v6. I just wanted to do this before we work on those new card/hero variants so that the icons are in the right place for those CTA links.

I actually found that a few of those icons don't need any vertical adjustments/margin-bottom so I'd rather have the default be null instead. Some of those with the -1px value look slightly misaligned.

Actually, @sherakama, now that I think about it, I'm going to change the $animate parameter since Kerri asked for a few different animation. In that case, maybe I'll keep the original parameter order 🤔

Yeah, V6 is planned for release after the next Palooza sprint which is in December. We will likely release a new version in January as we usually have a few items to wrap up after.

sherakama

Looks good, just one comment about API changes.

…ptions and apply to link variants; add lock-solid.svg for hover state for link--internal

yvonnetangsu · 2019-10-07T17:41:16Z

@sherakama , I added options to $animate per Kerri's request and swapped the order of $animate and $vertical so $animate is back to being the 3rd parameter. Could you please take another look? Is this still API breaking?

sherakama · 2019-10-08T16:49:19Z

core/src/scss/components/link/_link--internal.scss

+  &:hover,
+  &:focus {
+    &::after {
+      mask-image: url("#{$su-image-path}/lock-solid.svg");


Support looks good for this and the autoprefixer rendered the right prefixes.
https://caniuse.com/#search=mask-image

sherakama

GTG

* master: Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter (#504) Delete card-2019-08-28.twig (#506) Update _input.scss (#507) Scale down type-a and type-b font size at xs and sm breakpoints, repurpose splash font and minor typography tweaks (#498)

* master: (28 commits) Updated packages and configuration files. (#527) Use rem instead of em for big button font size so it doesn't blow up in sections if we set a large base font size; finetune big button padding for smaller breakpoints (#522) Change sizes of link icons to use px instead of em to prevent occasional clipping (#523) D8CORE-628: Add caption option to hero banner. (#517) Use percentage for html root font size instead of px so people can scale up using browser font size preferences (#526) Mixins documention update — @centered-column (#448) Adds Tugboat and fixes npm dependencies. (#518) Update _hero.scss (#513) Create Aspect Ratio Mixin (#445) Adding global footer variants and link tweaks for legibility, change layout at MD breakpoint to match Figma (#500) Updated package and lock files. Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter (#504) Delete card-2019-08-28.twig (#506) Update _input.scss (#507) Scale down type-a and type-b font size at xs and sm breakpoints, repurpose splash font and minor typography tweaks (#498) Add 4 new @modular-spacing steps (#6 to #9) and modify 2 existing steps (#3 and #4) (#503) add su- prefix to Decanter core and component variables, deprecate old names (#452) Includevar (#496) Put appearance back for select dropdowns (#479) Split input.scss into several files and add new input-base placeholder (#476) ... # Conflicts (resolved manually) # core/dist/css/decanter.css # core/src/scss/components/card/_card.scss # core/src/scss/components/card/index.scss # core/src/scss/utilities/placeholders/components/_card.scss # core/src/templates/components/card/card.twig

Add parameter to @link-icon to adjust vertical position of icon; fine…

771c43b

… tune existing link variants icon vertical positions

yvonnetangsu self-assigned this Oct 2, 2019

Use em unit instead of px

051af57

yvonnetangsu requested a review from sherakama October 2, 2019 17:32

yvonnetangsu added the enhancement label Oct 2, 2019

sherakama reviewed Oct 2, 2019

View reviewed changes

sherakama requested changes Oct 2, 2019

View reviewed changes

yvonnetangsu changed the title ~~Add parameter to @link-icon to adjust vertical position of icon~~ Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter Oct 3, 2019

Change $animate parameter in @link-icon to take different animation o…

373c4bc

…ptions and apply to link variants; add lock-solid.svg for hover state for link--internal

yvonnetangsu requested a review from sherakama October 7, 2019 17:38

sherakama reviewed Oct 8, 2019

View reviewed changes

sherakama added 2 commits October 8, 2019 09:50

Update _link-icon.scss

9f70057

Update _link-icon.scss

9f94215

sherakama approved these changes Oct 8, 2019

View reviewed changes

sherakama merged commit d829ae3 into master Oct 8, 2019

sherakama deleted the 456-link-icon-position branch October 8, 2019 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter #504

Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter #504

yvonnetangsu commented Oct 2, 2019 •

edited

Loading

sherakama Oct 2, 2019

yvonnetangsu Oct 3, 2019

yvonnetangsu Oct 3, 2019

sherakama Oct 3, 2019

sherakama left a comment

yvonnetangsu commented Oct 7, 2019

sherakama Oct 8, 2019

sherakama left a comment

Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter #504

Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter #504

Conversation

yvonnetangsu commented Oct 2, 2019 • edited Loading

READY FOR REVIEW

Summary

Needed By (Date)

Urgency

Steps to Test

Affected Projects or Products

Associated Issues and/or People

sherakama Oct 2, 2019

Choose a reason for hiding this comment

yvonnetangsu Oct 3, 2019

Choose a reason for hiding this comment

yvonnetangsu Oct 3, 2019

Choose a reason for hiding this comment

sherakama Oct 3, 2019

Choose a reason for hiding this comment

sherakama left a comment

Choose a reason for hiding this comment

yvonnetangsu commented Oct 7, 2019

sherakama Oct 8, 2019

Choose a reason for hiding this comment

sherakama left a comment

Choose a reason for hiding this comment

yvonnetangsu commented Oct 2, 2019 •

edited

Loading