Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: Use base fonts throughout the app #6881

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Dec 4, 2019

We've had a set of font %placeholders in our base styles for quite a while
but not bitten the bullet to use them. This begins to use them.

We had to make a small amount of tweaks to base whilst doing this, but
its as we'd prefer there to be as few font placeholders as possible. We
might/should be able to reduce these further at somepoint, or
potentially rename them. We currently have six header fonts (or 4 header
fonts/2 strong body fonts) and 3 body fonts.

We also noticed an empty CSS file and deleted that while we were here.
We also noticed that the bottom border of structure tabs was a pixel
larger than ours so we tweaked that here also.

Moving forwards we are likely to make some tweaks to our base files
to bring the 100% into line with structure. Our app typography.scss file
is now much smaller/saner.

Preview link.

Further down the line we are considering moving a lot of this into a
component/component-name/typography.scss file (so similar to the layout.scss/skin.scss files). We've quibbled
about doing this for a while as generally you want fonts to cascade/be
configured from the outside as much as possible and not have the component
itself dictate font sizes. If we do move any of this to the component specific file,
we'll probably make it optional so you still have the option of doing it at a project
level.

@johncowen johncowen requested review from a team, hannahhearth and jnwright December 4, 2019 17:23
@johncowen johncowen added the theme/ui Anything related to the UI label Dec 4, 2019
@johncowen johncowen force-pushed the feature/ui-use-base-fonts branch from 347b3b6 to 5850ff2 Compare December 11, 2019 10:20
@codecov
Copy link

codecov bot commented Dec 11, 2019

Codecov Report

Merging #6881 into ui-staging will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##           ui-staging   #6881      +/-   ##
=============================================
- Coverage       65.63%   65.6%   -0.03%     
=============================================
  Files             443     443              
  Lines           53292   53292              
=============================================
- Hits            34977   34964      -13     
- Misses          14096   14108      +12     
- Partials         4219    4220       +1
Impacted Files Coverage Δ
agent/router/manager.go 87.5% <0%> (-5.36%) ⬇️
agent/consul/session_ttl.go 85.48% <0%> (-3.23%) ⬇️
agent/consul/replication.go 85.71% <0%> (-2.39%) ⬇️
agent/consul/config_replication.go 75.51% <0%> (-2.05%) ⬇️
agent/kvs_endpoint.go 75.44% <0%> (-1.2%) ⬇️
api/watch/funcs.go 74.61% <0%> (-1.04%) ⬇️
agent/service_manager.go 81.62% <0%> (-0.86%) ⬇️
agent/consul/connect_ca_endpoint.go 56.52% <0%> (-0.67%) ⬇️
agent/consul/leader_connect.go 70.58% <0%> (-0.44%) ⬇️
agent/consul/leader.go 69.61% <0%> (-0.39%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0244c51...8fdde95. Read the comment docs.

We've had a set of %placeholders in our base styles for quite a while
but not butten the bullet to use them. This begins to use them.

We had to make a small amount of tweaks to base whilst doing this, but
its as we'd prefer there to be as few font placeholders as possible. We
might/should be able to reduce these further at somepoint, or
potentially rename them. We currently have six header fonts (or 4 header
fonts/2 strong body fonts) and 3 body fonts.

We also noticed an empty CSS file and deleted that while we were here.
We also noticed that the bottom border of structure tabs was a pixel
larger than ours so we tweaked that here also.
@johncowen johncowen force-pushed the feature/ui-use-base-fonts branch from 5850ff2 to 8fdde95 Compare December 11, 2019 11:24
@johncowen johncowen merged commit c3d2fe1 into ui-staging Dec 12, 2019
@johncowen johncowen deleted the feature/ui-use-base-fonts branch December 12, 2019 09:25
johncowen added a commit that referenced this pull request Dec 17, 2019
We've had a set of %placeholders in our base styles for quite a while
but not butten the bullet to use them. This begins to use them.

We had to make a small amount of tweaks to base whilst doing this, but
its as we'd prefer there to be as few font placeholders as possible. We
might/should be able to reduce these further at somepoint, or
potentially rename them. We currently have six header fonts (or 4 header
fonts/2 strong body fonts) and 3 body fonts.

We also noticed an empty CSS file and deleted that while we were here.
We also noticed that the bottom border of structure tabs was a pixel
larger than ours so we tweaked that here also.
johncowen added a commit that referenced this pull request Dec 18, 2019
We've had a set of %placeholders in our base styles for quite a while
but not butten the bullet to use them. This begins to use them.

We had to make a small amount of tweaks to base whilst doing this, but
its as we'd prefer there to be as few font placeholders as possible. We
might/should be able to reduce these further at somepoint, or
potentially rename them. We currently have six header fonts (or 4 header
fonts/2 strong body fonts) and 3 body fonts.

We also noticed an empty CSS file and deleted that while we were here.
We also noticed that the bottom border of structure tabs was a pixel
larger than ours so we tweaked that here also.
@ghost
Copy link

ghost commented Jan 25, 2020

Hey there,

This issue has been automatically locked because it is closed and there hasn't been any activity for at least 30 days.

If you are still experiencing problems, or still have questions, feel free to open a new one 👍.

@ghost ghost locked and limited conversation to collaborators Jan 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants