215 Logo lockup #252

yvonnetangsu · 2018-11-01T05:08:13Z

READY FOR REVIEW

Summary

Lockup component with Stanford wordmark logo and the site (unit/department) name.

Needed By (Date)

N/A

Urgency

N/A

Steps to Test

Pull this branch
Run grunt styleguide
Check if the lockup with the individual line variants match the design in the Figma file Stanford Design Library "Header, Footer & Navigation" page, some of the variants in the Stanford Open Framework site lockup guide, and the Identity toolkit page on department lockups.
Check behavior for smaller breakpoints.
Try changing values in lockup.json (eg, with or without line 2/5, different variants for line 1/5) and see if interplay works as expected (eg, horizontal line on mobile disappear when there is no line 5, variant of line 1 affects font size of line 2 etc).

Affected Projects or Products

Decanter

Associated Issues and/or People

Create lockup component (Twig & SASS) #215
Only including line 1, 2, and 5 in the site name for this version. Will add support for adding line 3 and 4 in future release.
Added conditionals in twig file to check for which variant of the lockup to use (default one-line or the two-line variant) - please let me know if this is a good approach.
I started with font-size: 30px (same as in Figma file Stanford Design Library "Header, Footer & Navigation" page - the word "Biology" next to wordmark) for line 1 regular variant, and eyeballed the rest of the font sizes based on the Stanford Open Framework site lockup guide.
With/without line 5 (at the bottom) isn't a variant since the grid simply collapses when it's not there.
The ordering of the elements in twig file is: wordmark, pipe, line 5, line 1, line 2 since I need to use the adjacent CSS selector + to have the line show up only when line 5 is present on mobile.
@sherakama (just FYI in case you want to comment on anything since the way I coded this was a bit experimental)

…or interplay between lines

…y if line 5 is present) and finetune font sizes; add conditionals in twig

* master: 216 Create site search component (#262) 266: Update scss file to reference logo.twig and update twig file to … (#267) 246 homepage (#258) reversed order of modular scale type mixin (#260) 122 hero (#259) 195 added all Stanford approved fonts (#248) change class for logo to match new naming conventions. (#257) px to rem (#256) 219 Modular typography (#250) fixup! wip. (#255) 79 card (#241) make color change on hover & focus less abrupt (#253) 249: Change CTA icon to a variable (#251) 223 brand bar (#243) # Conflicts: # core/css/decanter.css # core/scss/components/index.scss # core/templates/components/logo/logo.twig

core/templates/components/lockup/lockup.twig

Co-Authored-By: yvonnetangsu <[email protected]>

core/scss/components/lockup/_lockup.scss

josephgknox

Nice work! This is looking really great. I left a couple of comments, one of which was a question regarding variant inclusion in the style guide. LMK your thoughts.

josephgknox

Thanks for the update and the discussion. This looks good!

…content

yvonnetangsu · 2018-11-08T19:34:31Z

Just added one more thing to limit the clickable area of the lockup (only clickable where there is text).

core/scss/components/lockup/_lockup.scss

JBCSU · 2018-11-09T00:45:04Z

core/templates/components/lockup/lockup.twig

+ * - .su-lockup__sitename-line2 - Regular font style for line 2
+ * - .su-lockup__sitename-line5 - Regular font style for line 5
+ * - .su-lockup__sitename-line5-uppercase - Uppercase, bold and smaller font for line 5
+ *


IMO, .su-lockup, .su-lockup__sitename-line1, .su-lockup__sitename-line2 and .su-lockup__sitename-line5 are not variants. Those classes are required in all cases in order for the component to render correctly. And indeed they're hardcoded in the template. I would argue for removing them from their respective list of "Available variants." Yvonne said she documented them that way in order to be consistent with existing doc, but we couldn't (quickly) find an example of that. @josephgknox & @sherakama, your thoughts on whether or not these required classes should be documented as "variants"?

I see what you mean, @JBCSU . My proposed tweak would be to list the "defaults" in their own section, and then the variants. @josephgknox and @sherakama , LMK what you think.

I redid the comments in lockup.twig. LMK what you all think.

Thanks for bringing this up @JBCSU. I completely agree with you here. @yvonnetangsu thanks for addressing this already. My one (very small) comment would be to move the Default classes for the individual site name lines comment above the Available variant for the lockup comment so that class comments and variant comments are next to one another.

core/templates/components/lockup/lockup.twig

JBCSU · 2018-11-09T00:56:48Z

core/scss/components/lockup/_lockup.scss

+  line-height: .8;
+
+  @media #{$breakpoint-md} {
+    white-space: nowrap;


I think line 5 should be allowed to wrap at all breakpoints. Hopefully we'll never have a stupid long line 5 like I used to test, but if we do, there's no point making it look worse than it is.

Actually I tested line 1 with "FREEMAN SPOGLI INSTITUTE FOR INTERNATIONAL STUDIES", and I think line 1 needs to wrap at the bottom end of md as well.

Please see below

JBCSU · 2018-11-09T00:58:36Z

core/scss/components/lockup/_lockup.scss

+  -ms-grid-column: 1;
+  -ms-grid-column-span: 1;
+  color: $color-black;
+  line-height: .8;


Line 5 wraps at xs and sm breakpoints (I think it should be allowed to wrap at all breakpoints - see below). When it wraps, line-height: .8 really doesn't look good. Is it worth the effort to refactor this in case someone has a line 5 that wraps?

Actually, same comment applies to line 1. Try "FREEMAN SPOGLI INSTITUTE FOR INTERNATIONAL STUDIES" for line 1.

Probably the same for the other lines???

I made some tweaks so all lines can now wrap. I did refactor the line height/padding of line 5, but I'd rather leave the other lines alone since the pipe height kind of depends on the other line heights. Perhaps that would encourage people to not have super long site names that wrap to the 2nd line? 😄

And maybe they could use the 2 line variant if the site name needs 2 lines. They can have both default for line 1 and line 2 and it would look like it's a single line wrapped to the 2nd. Eg, FREEMAN SPOGLI INSTITUTE FOR INTERNATIONAL STUDIES works well this way.

Co-Authored-By: yvonnetangsu <[email protected]>

* master: 89 browsersync (#272) 189 Alert Component (#263) # Conflicts: # package-lock.json - resolved using master

JBCSU · 2018-11-21T20:12:12Z

core/scss/components/lockup/_lockup.scss

+}
+
+.su-lockup__sitename-line5--uppercase {
+  font-size: 18px;


I'm surprised by the extreme difference in font size between line 5's default (27px on desktop) and the uppercase variant (18px on desktop). But in looking at the Stanford Open Framework site lockup guide, I see why you did that. So I'm good with this now, but we may revisit later.

* master: Added composer.json Added composer.json 215 Logo lockup (#252) # Conflicts: # core/css/decanter.css # core/scss/components/index.scss

yvonnetangsu added 3 commits October 30, 2018 23:58

215: Initial setup of lockup files

0970f6f

215: Add code for line 2 and line 5 in the site name

9243828

215: Add variant for 2 lines site name to right of wordmark and CSS f…

e30ad87

…or interplay between lines

yvonnetangsu self-assigned this Nov 1, 2018

yvonnetangsu added 7 commits November 2, 2018 18:38

215: lockup media queries for smaller breakpoints

e0a3932

215: merge master and resolve conflict

4009e4e

215: Add conditional behavior for the horizontal divider (appears onl…

37470be

…y if line 5 is present) and finetune font sizes; add conditionals in twig

215: Codeclimate fix and grid touch up

0de811e

215: cc fix

1e0c5a6

215: Add -ms grid properties and fixup

c88342e

yvonnetangsu requested review from josephgknox and JBCSU November 7, 2018 22:59

josephgknox reviewed Nov 8, 2018

View reviewed changes

core/templates/components/lockup/lockup.twig Show resolved Hide resolved

215: Add attributes and modifier_class variables in twig comment

24eaadd

Co-Authored-By: yvonnetangsu <[email protected]>

josephgknox reviewed Nov 8, 2018

View reviewed changes

core/scss/components/lockup/_lockup.scss Show resolved Hide resolved

josephgknox reviewed Nov 8, 2018

View reviewed changes

josephgknox approved these changes Nov 8, 2018

View reviewed changes

yvonnetangsu added 2 commits November 8, 2018 11:05

215: fixup

9b150e0

215: Add width:fit-content to limit clickable area to where there is …

530ebfe

…content

JBCSU reviewed Nov 9, 2018

View reviewed changes

JBCSU and others added 6 commits November 8, 2018 17:16

215: fixup

e647433

Co-Authored-By: yvonnetangsu <[email protected]>

215: fixup

2652420

Co-Authored-By: yvonnetangsu <[email protected]>

215: tweak twig comments, line wrap and line height

95c2d6b

215: add styleguide comment

92c9b6e

Co-Authored-By: yvonnetangsu <[email protected]>

Merge branch 'master' into 215-lockup

71ccf08

* master: 89 browsersync (#272) 189 Alert Component (#263) # Conflicts: # package-lock.json - resolved using master

remove duplicated markup from lockup template

a7b91fc

JBCSU approved these changes Nov 21, 2018

View reviewed changes

JBCSU merged commit f6da6ad into master Nov 21, 2018

JBCSU deleted the 215-lockup branch November 21, 2018 20:15

yvonnetangsu added a commit that referenced this pull request Nov 26, 2018

Merge branch 'master' into 72-main-nav

0542fd0

* master: Added composer.json Added composer.json 215 Logo lockup (#252) # Conflicts: # core/css/decanter.css # core/scss/components/index.scss

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

215 Logo lockup #252

215 Logo lockup #252

yvonnetangsu commented Nov 1, 2018 •

edited

Loading

josephgknox left a comment

josephgknox left a comment

yvonnetangsu commented Nov 8, 2018

JBCSU Nov 9, 2018

yvonnetangsu Nov 9, 2018

yvonnetangsu Nov 9, 2018

josephgknox Nov 12, 2018

JBCSU Nov 9, 2018

JBCSU Nov 9, 2018

yvonnetangsu Nov 9, 2018

JBCSU Nov 9, 2018

JBCSU Nov 9, 2018

yvonnetangsu Nov 9, 2018

yvonnetangsu Nov 9, 2018 •

edited

Loading

JBCSU Nov 21, 2018

215 Logo lockup #252

215 Logo lockup #252

Conversation

yvonnetangsu commented Nov 1, 2018 • edited Loading

READY FOR REVIEW

Summary

Needed By (Date)

Urgency

Steps to Test

Affected Projects or Products

Associated Issues and/or People

josephgknox left a comment

Choose a reason for hiding this comment

josephgknox left a comment

Choose a reason for hiding this comment

yvonnetangsu commented Nov 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yvonnetangsu Nov 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yvonnetangsu commented Nov 1, 2018 •

edited

Loading

yvonnetangsu Nov 9, 2018 •

edited

Loading