-
Notifications
You must be signed in to change notification settings - Fork 14k
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
chore: Make font-weights themable, fix font faces #19236
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19236 +/- ##
==========================================
- Coverage 66.55% 66.54% -0.01%
==========================================
Files 1646 1645 -1
Lines 63617 63713 +96
Branches 6471 6503 +32
==========================================
+ Hits 42339 42398 +59
- Misses 19600 19632 +32
- Partials 1678 1683 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up |
@kgabryje Ephemeral environment spinning up at http://34.213.199.151:8080. Credentials are |
@jinghua-qa Can you please help us test this one? |
superset-frontend/packages/superset-ui-core/src/chart/components/NoResultsComponent.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/Styles.js
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the improvements!
Ephemeral environment shutdown and build artifacts deleted. |
* fix(fonts): Import all necessary font packages * Make html tags themable * Set bold font weight to 600, add medium font weight to theme * Replace hard coded font weights with theme variables * Change some font weight light elements to normal * Fix tests * Fix bug in pivot table * Address code review comments
* fix(fonts): Import all necessary font packages * Make html tags themable * Set bold font weight to 600, add medium font weight to theme * Replace hard coded font weights with theme variables * Change some font weight light elements to normal * Fix tests * Fix bug in pivot table * Address code review comments
SUMMARY
src/assets/stylesheets/less/fonts.less
, in fact only the fonts with normal font weight were available. That means that font weights other than 400 simply didn't work - weights from 200 to 500 were rendered as 400, bold font weights were extrapolated by the browser. This PR fixes that issue by explicitely importing font files with weights 200, 400, 500 and 600.<h1-6>
,<th>
and<strong>
all usefont-weight: 700
by default. I've overriden them globally to usefont-weight: 600
.supersetTheme
from 700 to 600, because of the reasons described above.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
The changes affect the whole product - everything should look as it did before (if you compare side-by-side with current look, there are slight differences in how bold texts look - that's because we didn't actually use Inter font's bold version before).
ADDITIONAL INFORMATION
CC @kasiazjc