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

perf: finish getting rid of uses of the Roboto font #26552

Merged
merged 2 commits into from
Sep 19, 2024
Merged

Conversation

davidmurdoch
Copy link
Contributor

@davidmurdoch davidmurdoch commented Aug 20, 2024

Description

We use two custom fonts in our typography CSS: Euclid Circular B and Roboto.

Roboto is a fallback font, but since both are custom fonts Roboto will never be activated by our default typography. It can be removed.

Roboto is only used in one other place in our CSS, but the selector looks to be not used anymore.

Lastly, Material UI uses Roboto by default for input elements and their labels. This PR updates our use of it to use "Euclid Circular B" and our usual fall back fonts. I don't particularly like the implementation (setting fontFamily as a string constant in JS), but I'm not sure how to do it any better (cc @georgewrmarshall ).

It saves about 0-2ms from the load time (compared to popup-defer-scripts branch), so it's not substantial by any stretch. It does save 1.4 megabytes, about 6%, from the dist zip (compared to popup-defer-scripts branch), which seems worthwhile.

Fixes: #26589

Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-extension-platform Extension Platform team label Aug 20, 2024
Copy link

codecov bot commented Aug 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.09%. Comparing base (d9e989e) to head (9aac394).
Report is 356 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #26552   +/-   ##
========================================
  Coverage    70.09%   70.09%           
========================================
  Files         1413     1413           
  Lines        49255    49256    +1     
  Branches     13768    13768           
========================================
+ Hits         34524    34525    +1     
  Misses       14731    14731           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [18fe552]
Page Load Metrics (75 ± 11 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint671861022814
domContentLoaded39157712412
load46157752411
domInteractive97930178
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 96 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@davidmurdoch davidmurdoch marked this pull request as ready for review August 20, 2024 22:10
@davidmurdoch davidmurdoch requested review from a team as code owners August 20, 2024 22:10
@@ -207,11 +207,6 @@
&__bold {
font-weight: bold;
}

&__tilde {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find any use of this class anywhere in our compiled code.

@HowardBraham HowardBraham added the team-tiger Tiger team (for tech debt reduction + performance improvements) label Aug 21, 2024
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

Excellent work thank you for this 💯 LGTM!

  • Checked TextField label in storybook to confirm no visual regressions ✅
  • Checked no instances of Roboto remain in codebase
Screenshot 2024-08-21 at 2 43 41 PM

Screenshot 2024-08-21 at 2 44 17 PM

Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [9aac394]
Page Load Metrics (73 ± 8 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint721521002311
domContentLoaded4210370189
load4810373178
domInteractive106027147
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 96 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@davidmurdoch davidmurdoch requested a review from a team September 19, 2024 14:23
@davidmurdoch davidmurdoch merged commit 4f58f14 into develop Sep 19, 2024
79 checks passed
@davidmurdoch davidmurdoch deleted the remove-roboto branch September 19, 2024 15:03
@github-actions github-actions bot locked and limited conversation to collaborators Sep 19, 2024
@metamaskbot metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 19, 2024
@metamaskbot metamaskbot added release-12.5.0 Issue or pull request that will be included in release 12.5.0 and removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 labels Sep 29, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-12.5.0 on PR. Adding release label release-12.5.0 on PR and removing other release labels(release-12.6.0), as PR was added to branch 12.5.0 when release was cut.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.5.0 Issue or pull request that will be included in release 12.5.0 team-extension-platform Extension Platform team team-tiger Tiger team (for tech debt reduction + performance improvements)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Remove Roboto Font to Improve Performance and Consistency
10 participants