8000 MWPW-165992 Fix Chart Font Family by meganthecoder · Pull Request #3685 · adobecom/milo · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

MWPW-165992 Fix Chart Font Family #3685

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 21, 2025
Merged

MWPW-165992 Fix Chart Font Family #3685

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85ed392
MWPW-165992 Fix Chart Font Family
meganthecoder Feb 12, 2025
5320b30
Merge branch 'stage' into chart-font
meganthecoder Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libs/blocks/chart/chartLightTheme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default (deviceSize) => {
const axisColor = '#767676';
const options = {
textStyle: {
fontFamily: 'Adobe Clean',
fontFamily: '"Adobe Clean", adobe-clean, "Trebuchet MS", sans-serif',
Copy link
Contributor
@narcis-radu narcis-radu Feb 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to set a font family here? Ideally we want to inherit from body, which has the font family defined.
Screenshot 2025-02-17 at 12 12 13
Screenshot 2025-02-17 at 12 12 45

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chart library will add a font family if you don't set any so inheriting from body won't work.

Copy link
Contributor
@narcis-radu narcis-radu Feb 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meganthecoder - using inherit should do the trick
Screenshot 2025-02-20 at 17 02 56

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@narcis-radu Right, of course. Sorry, I didn't see this until after it got merged to stage. Do you want me to create a follow-up PR to change to inherit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meganthecoder - only if it's not too much trouble. We don't change fonts that often, but if we do, it would be one less line to change if we use inherit. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #3742

fontSize: 16,
fontWeight: 700,
},
Expand Down
Loading
0