static/scss/base/_variables.scss - Issue 29485575: Issue 5385 - Create Site Header Component for Help Center

Side by Side Diff: static/scss/base/_variables.scss

Issue 29485575: Issue 5385 - Create Site Header Component for Help Center (Closed)

Patch Set: Fix alignment of search form, reformat JS Created Aug. 22, 2017, 12:14 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
1 // This file is part of help.eyeo.com.	1 // This file is part of help.eyeo.com.

2 // Copyright (C) 2017 Eyeo GmbH	2 // Copyright (C) 2017 Eyeo GmbH

3 //	3 //

4 // help.eyeo.com is free software: you can redistribute it and/or modify	4 // help.eyeo.com is free software: you can redistribute it and/or modify

5 // it under the terms of the GNU General Public License as published by	5 // it under the terms of the GNU General Public License as published by

6 // the Free Software Foundation, either version 3 of the License, or	6 // the Free Software Foundation, either version 3 of the License, or

7 // (at your option) any later version.	7 // (at your option) any later version.

8 //	8 //

9 // help.eyeo.com is distributed in the hope that it will be useful,	9 // help.eyeo.com is distributed in the hope that it will be useful,

10 // but WITHOUT ANY WARRANTY; without even the implied warranty of	10 // but WITHOUT ANY WARRANTY; without even the implied warranty of

(...skipping 33 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
44	44

45 // Font sizes (headings)	45 // Font sizes (headings)

46	46

47 $font-size-h1: 3em;	47 $font-size-h1: 3em;

48 $font-size-h2: 2em;	48 $font-size-h2: 2em;

49 $font-size-h3: 1.4em;	49 $font-size-h3: 1.4em;

50 $font-size-h4: 1em;	50 $font-size-h4: 1em;

51 $font-size-h5: $font-size-h4;	51 $font-size-h5: $font-size-h4;

52 $font-size-h6: $font-size-h4;	52 $font-size-h6: $font-size-h4;

53	53

	54 // Font sizes (general)

	55

	56 $small-font: 0.9em;

	57 $tiny-font: 0.8em;

	58

54 // Font weights	59 // Font weights

55	60

56 $bold-weight: 700;	61 $bold-weight: 700;

57	62

58 // Sizes ///////////////////////////////////////////////////////////////////////	63 // Sizes ///////////////////////////////////////////////////////////////////////

59	64

	65 // Container widths

	66

	67 $content-width: $desktop-width;

	68 $navbar-width: $content-width + 100px;
	juliandoucette 2017/08/23 14:39:07 I think it's a better idea to start from the $desk I think it's a better idea to start from the $desktop-width and move inwards instead of outwards. e.g. use .container inside .navbar instead of .navbar-wrapper and make $content-width = $desktop-width - 100px. ire 2017/08/25 08:27:10 I like the idea of moving inwards instead of outwa Show quoted text On 2017/08/23 14:39:07, juliandoucette wrote: > I think it's a better idea to start from the $desktop-width and move inwards > instead of outwards. e.g. use .container inside .navbar instead of > .navbar-wrapper and make $content-width = $desktop-width - 100px. I like the idea of moving inwards instead of outwards, but I think the .container class is a bit restrictive. Since it uses fixed widths, it doesn't really take advantage of the space available like the current implementation does. There are viewport sizes in which the container could/should be wider, but maintains a smaller fixed width. juliandoucette 2017/08/29 16:17:27 Acknowledged. I see what you mean now. And I agre Show quoted text On 2017/08/25 08:27:10, ire wrote: > I like the idea of moving inwards instead of outwards, but I think the > .container class is a bit restrictive. Since it uses fixed widths, it doesn't > really take advantage of the space available like the current implementation > does. There are viewport sizes in which the container could/should be wider, but > maintains a smaller fixed width. Acknowledged. I see what you mean now. And I agree with your implementation. It may be useful to add a comment about this. I'll leave it up to you to decide whether you think this is necessary. ire 2017/09/01 10:57:18 I don't think my comment would be more explanatory Show quoted text On 2017/08/29 16:17:27, juliandoucette wrote: > On 2017/08/25 08:27:10, ire wrote: > > I like the idea of moving inwards instead of outwards, but I think the > > .container class is a bit restrictive. Since it uses fixed widths, it doesn't > > really take advantage of the space available like the current implementation > > does. There are viewport sizes in which the container could/should be wider, > but > > maintains a smaller fixed width. > > Acknowledged. > > I see what you mean now. And I agree with your implementation. It may be useful > to add a comment about this. I'll leave it up to you to decide whether you think > this is necessary. I don't think my comment would be more explanatory than "navbar width is content width plus 100px", so I'll leave it out
	69

	70 // Header & Footer

	71

	72 $site-header-height: 70px;
	juliandoucette 2017/08/23 14:39:07 If these variables only apply to the site header l If these variables only apply to the site header like their name implies then I think we can move them to _header.scss. The same applies to $search-form-* below. ire 2017/08/25 08:27:11 I'm undecided on whether it's better to have all t Show quoted text On 2017/08/23 14:39:07, juliandoucette wrote: > If these variables only apply to the site header like their name implies then I > think we can move them to _header.scss. The same applies to $search-form-* > below. I'm undecided on whether it's better to have all the variables in one file, or to have them closer to where they are used. One reason I kept the $site-header-height variable in this file in particular was because the variable was also used in the layout file (when the header was stuck to the top). Even though this is the $site-header-height, I can imagine cases where the variable may be referenced elsewhere (and it would still be correct to call it $site-header-height). Also, I think it's easier to change (and share) when all variables are in one file. juliandoucette 2017/08/29 16:17:26 Acknowledged. I concede this point. Show quoted text On 2017/08/25 08:27:11, ire wrote: > On 2017/08/23 14:39:07, juliandoucette wrote: > > If these variables only apply to the site header like their name implies then > I > > think we can move them to _header.scss. The same applies to $search-form-* > > below. > > I'm undecided on whether it's better to have all the variables in one file, or > to have them closer to where they are used. One reason I kept the > $site-header-height variable in this file in particular was because the variable > was also used in the layout file (when the header was stuck to the top). Even > though this is the $site-header-height, I can imagine cases where the variable > may be referenced elsewhere (and it would still be correct to call it > $site-header-height). Also, I think it's easier to change (and share) when all > variables are in one file. Acknowledged. I concede this point.
	73 $site-header-content-line-height: 40px;

	74

	75 // Search form

	76

	77 $search-form-width: 320px;

	78 $search-form-height: 35px;

	79

60 // Spacers /////////////////////////////////////////////////////////////////////	80 // Spacers /////////////////////////////////////////////////////////////////////

61	81

	82 $xl: 60px;

	83 $lg: 30px;

	84 $md: 15px;

	85 $sm: 10px;

	86 $xs: 5px;

	87

62 // Columns /////////////////////////////////////////////////////////////////////	88 // Columns /////////////////////////////////////////////////////////////////////

63	89

64 // Breakpoints /////////////////////////////////////////////////////////////////	90 // Breakpoints /////////////////////////////////////////////////////////////////

65	91

66 // Placeholders ////////////////////////////////////////////////////////////////	92 $mobile-breakpoint: 544px;
	juliandoucette 2017/08/23 14:39:06 The website-default grid considers anything less t The website-default grid considers anything less than tablet-breakpoint to be mobile. We don't currently support phablets (less than 768px and more than 544px) differently than mobile devices (between 320px - 544px). ire 2017/08/25 08:27:09 What is the reason for not having a difference? Be Show quoted text On 2017/08/23 14:39:06, juliandoucette wrote: > The website-default grid considers anything less than tablet-breakpoint to be > mobile. We don't currently support phablets (less than 768px and more than > 544px) differently than mobile devices (between 320px - 544px). What is the reason for not having a difference? Because those are significant differences in size, a mobile phone versus an iPad, for example. I think at the size of tablets (greater than 544px), there is still a lot of screen space to be used and switching to mobile styles (full widths etc) may be underutilising the space available. Also, I think the variable names in themselves are more for us to to easily understand the size, and could as easily be something like $small-breakpoint, $medium-breakpoint, and $large-breakpoint. juliandoucette 2017/08/29 16:17:26 There seems to be a miscommunication here about wh Show quoted text On 2017/08/25 08:27:09, ire wrote: > What is the reason for not having a difference? Because those are significant > differences in size, a mobile phone versus an iPad, for example. I think at the > size of tablets (greater than 544px), there is still a lot of screen space to be > used and switching to mobile styles (full widths etc) may be underutilising the > space available. There seems to be a miscommunication here about where a phone ends and a tablet begins. You are saying that a phone ends and a tablet begins at 544. I am saying that a phone ends and a tablet begins at 768. I chose the tablet breakpoint (minimum width) (768) based on [the width of the browser screen on the first ipad when held portrait, the width at which you can comfortably fit two columns in most layouts]. I will certainly acknowledge that there is room between 544 and 768 for optimization. And encourage you to optimize for this size (which I would call "phablet size") if you can. (I didn't include a phablet width or breakpoint in website-defaults grid because none of our existing website designs explicitly support these sizes.) Show quoted text > Also, I think the variable names in themselves are more for us to to easily > understand the size, and could as easily be something like $small-breakpoint, > $medium-breakpoint, and $large-breakpoint. I agree. But I'm not sure that [small, medium, large] are better than [mobile, tablet, desktop]; yet? ire 2017/09/01 10:57:19 Changed to $phablet-breakpoint Show quoted text On 2017/08/29 16:17:26, juliandoucette wrote: > On 2017/08/25 08:27:09, ire wrote: > > What is the reason for not having a difference? Because those are significant > > differences in size, a mobile phone versus an iPad, for example. I think at > the > > size of tablets (greater than 544px), there is still a lot of screen space to > be > > used and switching to mobile styles (full widths etc) may be underutilising > the > > space available. > > There seems to be a miscommunication here about where a phone ends and a tablet > begins. You are saying that a phone ends and a tablet begins at 544. I am saying > that a phone ends and a tablet begins at 768. > > I chose the tablet breakpoint (minimum width) (768) based on [the width of the > browser screen on the first ipad when held portrait, the width at which you can > comfortably fit two columns in most layouts]. > > I will certainly acknowledge that there is room between 544 and 768 for > optimization. And encourage you to optimize for this size (which I would call > "phablet size") if you can. > > (I didn't include a phablet width or breakpoint in website-defaults grid because > none of our existing website designs explicitly support these sizes.) > > > Also, I think the variable names in themselves are more for us to to easily > > understand the size, and could as easily be something like $small-breakpoint, > > $medium-breakpoint, and $large-breakpoint. > > I agree. But I'm not sure that [small, medium, large] are better than [mobile, > tablet, desktop]; yet? Changed to $phablet-breakpoint
	93

	94 // Placeholders ////////////////////////////////////////////////////////////////

OLD	NEW

« static/scss/base/_utilities.scss ('K') | « static/scss/base/_utilities.scss ('k') | static/scss/components/_search-form.scss » ('j') | static/scss/components/_search-form.scss » ('J')