Removed 'Muli' font. Added 'Manrope' font. Extended 'Poppins' with 'Manrope' for Cyrillic&Greek chars. #12933

boocmp · 2022-04-07T14:18:04Z

Resolves brave/brave-browser#4309
Resolves brave/brave-browser#25527

Submitter Checklist:

I confirm that no security/privacy review is needed, or that I have requested one
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

On respective Issues

brave-builds · 2022-04-07T15:14:44Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-04-10T04:24:01Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-04-11T02:17:30Z

A Storybook has been deployed to preview UI for the latest push

iefremov · 2022-04-21T18:19:56Z

kindly ping @petemill @bsclifton @zenparsing - can you please take a look when you have some time? This fixes a P2 issue

iefremov · 2022-04-21T18:20:10Z

@boocmp needs rebase

petemill · 2022-04-21T18:23:28Z

build/rust/config.gni

@@ -47,7 +47,8 @@ if (use_lld) {

 # https://bugzilla.mozilla.org/show_bug.cgi?id=1188030#c14
 # Supposedly this bug was fixed, but it's still happening for us.
-too_many_personality_profiles_workaround = is_apple && use_lld && !is_component_build


why is this being changed in this PR?

linter has gone crazy :(

Because this PR is modifying a lot of places, I'd like to propose backing out this lint change (and any any others not absolutely needed). If the file was being modified already, then lint change makes sense. But general lint cleanups can happen in a different PR

petemill · 2022-04-21T18:29:32Z

I'll take a look at reviewing this, but I'll need to do a build to see the difference. I would suggest you append some more items the description:

Which UIs that are affected by replacing Multi with Poppins, so we can check font-size and spacing
Which different UIs in particular to look at for cyrillic improvements, if any
Any other test scenarios that we should run by the design team

Some before and after screenshots would be helpful too.

Thanks!

bsclifton · 2022-04-22T07:56:30Z

Oh wow - this'll be a great one! (adding Cyrillic support). I can pull down and try after it's rebased

++ on everything @petemill mentioned 😄 Many of the UIs may look the same before/after and it's fine to skip screenshots. But for ones which have a difference (hopefully not a big spacing difference or line height difference, etc) it's good to see what that could be

brave-builds · 2022-04-22T09:55:46Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-04-22T10:37:58Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-04-22T11:45:47Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-04-23T03:58:33Z

A Storybook has been deployed to preview UI for the latest push

boocmp · 2022-04-23T04:39:18Z

Fonts.zip
See in the attachment a few screenshots for the English and Cyrillic versions.
In English version, Muli is replaced by Poppins, also all Cyrillic characters are displayed in Manrope
In Cyrillic version, only Manrope is used

petemill · 2022-04-25T19:44:11Z

Yeah it's looking pretty good at first glance of the screenshots. We'll want to get a designer to look over them to check the font-weight and size differences especially between Muli and Poppins. I think this is a good way of doing it since it'll improve cyrillic font display even for users who are currently viewing pages in non-cyrillic languages. Only issue may be that we have to be granular when choosing a font for different character sets.

Still some TODOs:

we removed muli.css but not the font files.
we have not removed all references to muli. Some references are provided by theme.fontFamily.body and our default theme provided by brave-ui has a value for this property as Muli. We can replace that value in brave-ui, which seems better semantics than replacing fontFamily.body with fontFamily.heading, although this older design-system is being deprecated soon.

In general we have way too many css definitions of font-family. It only really needs it once per UI since the css value is inherited!

iefremov · 2022-04-26T13:35:36Z

@jenn-rhim @bradleyrichter can you please look at screenshots and confirm they look good? #12933 (comment)

brave-builds · 2022-04-26T14:55:33Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-04-29T02:37:13Z

A Storybook has been deployed to preview UI for the latest push

- Remove Muli in favor of Poppins

…emove muli from more places

petemill · 2022-09-21T07:04:03Z

I've pushed a rebase and now that Poppins is correctly replacing instances of the brave-ui theme variable that resulted in Muli being requested via css, I've manually tweaked some of the items in #12933 (comment) - font weights and sizes.

I basically changed many instances of font-weight: 600 to font-weight: 500 for the Rewards Page, and it now looks like this (Poppins on left, old muli on right):

I think it's best to merge this and then cover anything additional in follow-up. I think I got the main items with Welcome and Rewards Page, and Brave Talk widget on NTP.

brave-builds · 2022-09-21T08:25:14Z

A Storybook has been deployed to preview UI for the latest push

zenparsing

Rewards CSS changes look good.

petemill · 2022-09-21T16:58:04Z

Confirmed that the change in brave-ui ref in package.json only brings in the Muli replacement with Poppins and does not remove any more recent commits from brave-ui. brave/brave-ui@8339e7d...e4c0498

…anrope' for Cyrillic&Greek chars. (#12933) * Reference to Manrope (Cyrillic & Greek) added for the Poppins font. * License. * Bye, bye 'Muli' it's a Poppin's time. * Overrided Poppins with Manrope. * Load font from resources instead of hardcode. * Lint. * GN fixed. * Lint. * Format. * Clean up. * Removed Mult files. * Font weight fixes. * Safety check padding. * Font-weight on active state. * Added comment. * Update brave-ui ref to include brave/brave-ui#607 - Remove Muli in favor of Poppins * Rebase. * Reverted package and package-lock jsons. * Rebased. * extra muli -> poppins text styles as a result of brave-ui update to remove muli from more places Co-authored-by: Pete Miller <miller.pete@gmail.com>

github-actions bot assigned boocmp Apr 7, 2022

github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Apr 7, 2022

boocmp force-pushed the issues/4309 branch from 6b2dfed to 7744126 Compare April 7, 2022 14:23

boocmp requested review from iefremov and petemill April 7, 2022 15:07

boocmp requested a review from a team as a code owner April 10, 2022 03:49

boocmp requested review from bsclifton and zenparsing April 10, 2022 03:49

boocmp force-pushed the issues/4309 branch from bf1016b to e1622d6 Compare April 10, 2022 03:53

boocmp changed the title ~~Reference to Manrope (Cyrillic & Greek) added for the Poppins font.~~ Removed 'Muli' font. Added 'Manrope' font. Extended 'Poppins' with 'Manrope' for Cyrillic&Greek chars. Apr 10, 2022

petemill reviewed Apr 21, 2022

View reviewed changes

boocmp force-pushed the issues/4309 branch from e456ecc to b285a5b Compare April 22, 2022 09:07

boocmp requested a review from a team as a code owner April 26, 2022 14:03

boocmp force-pushed the issues/4309 branch from 1f61ac0 to 4c9906a Compare April 29, 2022 02:08

boocmp and others added 18 commits September 20, 2022 17:00

Bye, bye 'Muli' it's a Poppin's time.

2db19fc

Overrided Poppins with Manrope.

54722e0

Load font from resources instead of hardcode.

78310c2

Lint.

60c33fc

GN fixed.

587853c

Lint.

a58e3c1

Format.

7667fd1

Clean up.

03f7537

Removed Mult files.

973a3a7

Font weight fixes.

9038284

Safety check padding.

bd1df3c

Font-weight on active state.

7c79875

Added comment.

f3d1ae8

Update brave-ui ref to include brave/brave-ui#607

dd52095

- Remove Muli in favor of Poppins

Rebase.

97a5e2b

Reverted package and package-lock jsons.

81bb377

Rebased.

75adb91

extra muli -> poppins text styles as a result of brave-ui update to r…

b12feeb

…emove muli from more places

petemill force-pushed the issues/4309 branch from ce5eb0c to b12feeb Compare September 21, 2022 07:04

petemill approved these changes Sep 21, 2022

View reviewed changes

zenparsing approved these changes Sep 21, 2022

View reviewed changes

petemill merged commit 096865a into master Sep 21, 2022

petemill deleted the issues/4309 branch September 21, 2022 16:58

github-actions bot added this to the 1.46.x - Nightly milestone Sep 21, 2022

petemill self-assigned this Sep 21, 2022

diracdeltas mentioned this pull request Nov 9, 2022

Cyrillic font improvement brave/brave-browser#26510

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed 'Muli' font. Added 'Manrope' font. Extended 'Poppins' with 'Manrope' for Cyrillic&Greek chars. #12933

Removed 'Muli' font. Added 'Manrope' font. Extended 'Poppins' with 'Manrope' for Cyrillic&Greek chars. #12933

boocmp commented Apr 7, 2022 •

edited by petemill

brave-builds commented Apr 7, 2022

brave-builds commented Apr 10, 2022

brave-builds commented Apr 11, 2022

iefremov commented Apr 21, 2022

iefremov commented Apr 21, 2022

petemill Apr 21, 2022

boocmp Apr 22, 2022

bsclifton Apr 22, 2022

boocmp Apr 23, 2022

petemill commented Apr 21, 2022

bsclifton commented Apr 22, 2022

brave-builds commented Apr 22, 2022

brave-builds commented Apr 22, 2022

brave-builds commented Apr 22, 2022

brave-builds commented Apr 23, 2022

boocmp commented Apr 23, 2022

petemill commented Apr 25, 2022 •

edited

iefremov commented Apr 26, 2022

brave-builds commented Apr 26, 2022

brave-builds commented Apr 29, 2022

petemill commented Sep 21, 2022

brave-builds commented Sep 21, 2022

zenparsing left a comment

petemill commented Sep 21, 2022

Removed 'Muli' font. Added 'Manrope' font. Extended 'Poppins' with 'Manrope' for Cyrillic&Greek chars. #12933

Removed 'Muli' font. Added 'Manrope' font. Extended 'Poppins' with 'Manrope' for Cyrillic&Greek chars. #12933

Conversation

boocmp commented Apr 7, 2022 • edited by petemill

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

brave-builds commented Apr 7, 2022

brave-builds commented Apr 10, 2022

brave-builds commented Apr 11, 2022

iefremov commented Apr 21, 2022

iefremov commented Apr 21, 2022

petemill Apr 21, 2022

Choose a reason for hiding this comment

boocmp Apr 22, 2022

Choose a reason for hiding this comment

bsclifton Apr 22, 2022

Choose a reason for hiding this comment

boocmp Apr 23, 2022

Choose a reason for hiding this comment

petemill commented Apr 21, 2022

bsclifton commented Apr 22, 2022

brave-builds commented Apr 22, 2022

brave-builds commented Apr 22, 2022

brave-builds commented Apr 22, 2022

brave-builds commented Apr 23, 2022

boocmp commented Apr 23, 2022

petemill commented Apr 25, 2022 • edited

iefremov commented Apr 26, 2022

brave-builds commented Apr 26, 2022

brave-builds commented Apr 29, 2022

petemill commented Sep 21, 2022

brave-builds commented Sep 21, 2022

zenparsing left a comment

Choose a reason for hiding this comment

petemill commented Sep 21, 2022

boocmp commented Apr 7, 2022 •

edited by petemill

petemill commented Apr 25, 2022 •

edited