Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

add windows specific font CSS for URL text and tab text #5624

Closed
bradleyrichter opened this issue Nov 15, 2016 · 11 comments
Closed

add windows specific font CSS for URL text and tab text #5624

bradleyrichter opened this issue Nov 15, 2016 · 11 comments
Assignees
Labels
design A design change, especially one which needs input from the design team. polish Nice to have — usually related to front-end/visual tasks. QA/checked-Linux QA/checked-macOS QA/checked-Win32 QA/checked-Win64 QA/test-plan-specified release-notes/include
Milestone

Comments

@bradleyrichter
Copy link
Contributor

bradleyrichter commented Nov 15, 2016

Test plan:

#7316 (comment)

URL bar

  1. Open the browser on Windows
  2. Make sure the URL bar looks like the mockup provided by @bradleyrichter
  3. Open https://www.apple.com
  4. Make sure the margin exists between the lock icon and the URL
  5. Open http://apple.com (not HTTPS)
  6. Make sure the same margin exists between the unlock icon and the URL
  7. Input ":g" on the URL bar
  8. Make sure the same margin exists between the Google icon and the search queries

I found some changes needed to improve legibility and sync the styles between mac and windows.

With the changes below, we will have a match between OSs in the URL and Tab text.

image

(note: these are for WINDOWS CSS only)

URL

  • increase font-weight to 500
  • move up 1 px by reducing top margin

image

update: this was fixed with #6848

tab font

  • change font weight to 500
  • change font size to 12px
  • change height to 18px
  • change color to match URL color

image

image

@bradleyrichter bradleyrichter added design A design change, especially one which needs input from the design team. polish Nice to have — usually related to front-end/visual tasks. labels Nov 15, 2016
@bradleyrichter bradleyrichter added this to the 0.12.10 release milestone Nov 15, 2016
@bbondy bbondy modified the milestones: 0.12.11, 0.12.10 release Nov 15, 2016
@srirambv
Copy link
Collaborator

@bradleyrichter +1 from support on Windows fonts being blurry compared to Chrome

@srirambv
Copy link
Collaborator

URL:
Font is still set at 400 and not 500, margin is up by 1px
image

Tab
Font weight is up to 500
Font size is up to 12px
Height changed to 18px
Tab color is not inherited from div.tab.active, is set to black
image

@luixxiul
Copy link
Contributor

@srirambv See: #6149 /cc: @bradleyrichter

@luixxiul
Copy link
Contributor

luixxiul commented Jan 16, 2017

Test plan

make sure that UI looks like the mockup image above #5624 (comment)

@luixxiul
Copy link
Contributor

luixxiul commented Jan 22, 2017

https://github.com/brave/browser-laptop/blame/2a78248d7b3e8611d251c2ef697f309f26d3a7c3/less/navigationBar.less#L40-L43

The 1st line needs to be .urlbarForm input by redoing #5752.

It is needed by this line https://github.com/brave/browser-laptop/blame/2a78248d7b3e8611d251c2ef697f309f26d3a7c3/less/navigationBar.less#L813 of my commit: 19360c1

  • increase font-weight to 500
  • move up 1 px by reducing top margin

@luixxiul
Copy link
Contributor

I reopened for the next milestone 0.13.1

@luixxiul
Copy link
Contributor

luixxiul commented Jan 22, 2017

By setting font-weight: 500 and margin: -1px 0 0 3px (as position: relative is inherited) correctly you will get:

clipboard01

However, by replacing the margin property with top: 0 to reset top: 1px and remove the gap between the fa-lock icon and the URL, you will get:

0px

Because fa-lock and fa-unlock icon is also aligned at the center of .urlbarIconContainer thanks to #6292 (comment), I think we no longer need the margin-left: 3px.

If you remove margin-left on 0.12.15, you get:
clipboard01
I think the margin-left was added to avoid that.

On the other hand, on 0.13.0 RC4 with the modification, you would get:
clipboard02

So I think we just need to set font-weight: 500 and top: 0 for .platform--win32 #navigator .urlbarForm input, removing margin: 1px 0 0 3px.

@bradleyrichter wdyt?

@bradleyrichter
Copy link
Contributor Author

@luixxiul looks good!

@luixxiul
Copy link
Contributor

@bradleyrichter OK I'll create a PR later ..

@luixxiul
Copy link
Contributor

luixxiul commented Feb 1, 2017

@bsclifton would you mind taking the tab font issue?

const activeTabStyle = {}
const backgroundColor = this.props.paintTabs && (this.props.tab.get('themeColor') || this.props.tab.get('computedThemeColor'))
if (this.props.isActive && backgroundColor) {
activeTabStyle.background = backgroundColor
const textColor = getTextColorForBackground(backgroundColor)
iconStyle.color = textColor
if (textColor) {
activeTabStyle.color = getTextColorForBackground(backgroundColor)
}
}

@bsclifton
Copy link
Member

bsclifton commented Feb 25, 2017

The remainder of the work has already been completed by @cezaraugusto (great job!) 🎉

The weight, pulling styles into Aphrodite, and more was done with https://github.com/brave/browser-laptop/pull/6900/files#diff-400b4094e3a5aaea84160dcc1483b3c2R250

And then the font size was addressed for all OSes with #7316

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design A design change, especially one which needs input from the design team. polish Nice to have — usually related to front-end/visual tasks. QA/checked-Linux QA/checked-macOS QA/checked-Win32 QA/checked-Win64 QA/test-plan-specified release-notes/include
Projects
None yet
Development

No branches or pull requests

6 participants