Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1291)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 8 months ago by ire
Modified:
1 year, 8 months ago
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
1 year, 8 months ago (2017-10-24 08:40:33 UTC) #1
ire
You're right, semibold looks way better :)
1 year, 8 months ago (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 ...
1 year, 8 months ago (2017-10-24 09:56:09 UTC) #3
ire
1 year, 8 months ago (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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5