Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code
Issues
All Issues

Issue 29587562: Issue 5903 - Change help.eyeo.com bold-weight from 700 to a semi-bold weight (Closed)

Created:
Oct. 24, 2017, 8:40 a.m. by ire
Modified:
Oct. 24, 2017, 2:44 p.m.
Reviewers:
juliandoucette
Base URL:
https://hg.adblockplus.org/help.eyeo.com
Visibility:
Public.

Description

Issue 5903 - Change help.eyeo.com bold-weight from 700 to a semi-bold weight

Patch Set 1 #

Total comments: 6

Patch Set 2 : Remove light font weight #

Unified diffs Side-by-side diffs Delta from patch set Stats (+111 lines, -904 lines) Patch
R static/fonts/Source-Sans-Pro-300/LICENSE.txt View 1 1 chunk +0 lines, -93 lines 0 comments Download
R static/fonts/Source-Sans-Pro-300/Source-Sans-Pro-300.eot View 1 Binary file 0 comments Download
R static/fonts/Source-Sans-Pro-300/Source-Sans-Pro-300.svg View 1 1 chunk +0 lines, -347 lines 0 comments Download
R static/fonts/Source-Sans-Pro-300/Source-Sans-Pro-300.ttf View 1 Binary file 0 comments Download
R static/fonts/Source-Sans-Pro-300/Source-Sans-Pro-300.woff View 1 Binary file 0 comments Download
R static/fonts/Source-Sans-Pro-300/Source-Sans-Pro-300.woff2 View 1 Binary file 0 comments Download
A static/fonts/Source-Sans-Pro-600/LICENSE.txt View 1 chunk +93 lines, -0 lines 0 comments Download
A static/fonts/Source-Sans-Pro-600/Source-Sans-Pro-600.eot View Binary file 0 comments Download
A static/fonts/Source-Sans-Pro-600/Source-Sans-Pro-600.ttf View Binary file 0 comments Download
A static/fonts/Source-Sans-Pro-600/Source-Sans-Pro-600.woff View Binary file 0 comments Download
A static/fonts/Source-Sans-Pro-600/Source-Sans-Pro-600.woff2 View Binary file 0 comments Download
R static/fonts/Source-Sans-Pro-700/LICENSE.txt View 1 chunk +0 lines, -93 lines 0 comments Download
R static/fonts/Source-Sans-Pro-700/Source-Sans-Pro-700.eot View Binary file 0 comments Download
R static/fonts/Source-Sans-Pro-700/Source-Sans-Pro-700.svg View 1 chunk +0 lines, -337 lines 0 comments Download
R static/fonts/Source-Sans-Pro-700/Source-Sans-Pro-700.ttf View Binary file 0 comments Download
R static/fonts/Source-Sans-Pro-700/Source-Sans-Pro-700.woff View Binary file 0 comments Download
R static/fonts/Source-Sans-Pro-700/Source-Sans-Pro-700.woff2 View Binary file 0 comments Download
M static/scss/base/_font.scss View 1 1 chunk +17 lines, -33 lines 0 comments Download
M static/scss/base/_variables.scss View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4
ire
Oct. 24, 2017, 8:40 a.m. (2017-10-24 08:40:33 UTC) #1
ire
You're right, semibold looks way better :)
Oct. 24, 2017, 8:40 a.m. (2017-10-24 08:40:54 UTC) #2
juliandoucette
LGTM https://codereview.adblockplus.org/29587562/diff/29587563/static/scss/base/_font.scss File static/scss/base/_font.scss (right): https://codereview.adblockplus.org/29587562/diff/29587563/static/scss/base/_font.scss#newcode24 static/scss/base/_font.scss:24: local("Source Sans Pro Light"), NIT: Are these spaces ...
Oct. 24, 2017, 9:56 a.m. (2017-10-24 09:56:09 UTC) #3
ire
Oct. 24, 2017, 2:43 p.m. (2017-10-24 14:43:57 UTC) #4 
https://codereview.adblockplus.org/29587562/diff/29587563/static/scss/base/_f...
File static/scss/base/_font.scss (right):

https://codereview.adblockplus.org/29587562/diff/29587563/static/scss/base/_f...
static/scss/base/_font.scss:24: local("Source Sans Pro Light"),
On 2017/10/24 09:56:08, juliandoucette wrote:
> NIT: Are these spaces tabs?

Nope, they are spaces

https://codereview.adblockplus.org/29587562/diff/29587563/static/scss/base/_f...
static/scss/base/_font.scss:27:
url("../fonts/Source-Sans-Pro-300/Source-Sans-Pro-300.woff") format("woff"),
On 2017/10/24 09:56:08, juliandoucette wrote:
> NIT: Are we using 300 anywhere?

Doesn't look like we are. Removed it too

https://codereview.adblockplus.org/29587562/diff/29587563/static/scss/base/_f...
static/scss/base/_font.scss:51: font-style: normal;
On 2017/10/24 09:56:08, juliandoucette wrote:
> NIT: You could name this "bold"
> Ack: That may be misleading, I'm undecided

Do you mean the "font-style"? That's to do with it being italic or nomal.

Also, bold would be incorrect, in case we ever decide to support actual bold.

Powered by Google App Engine
This is Rietveld