Yes please! Great idea!

Thank you for taking on this PR 😄

A couple of notes:

• Currently material icons are being loaded from the index.html file. Lets do the same thing here but load our local font instead
• Ideally we want to use this local material icons font for our extension popups as well. Right now they try to load in material icons from https://fonts.googleapis.com/icon?family=Material+Icons. Can we try replacing these with this local font as well? See devtools/projects/shell-browser/src/popups/
• third_party/github.com/google/material-design-icons/BUILD.bazel exports the LICENSE for material-design-icons as well. I think we should include that in the build in the same directory that our font is located in.

@AleksanderBodurri thanks for reviewing this PR and catching the typo!

Will follow your notes to double-check everything. Regarding the license file, are you suggesting adding the path to that license file into the filegroup within this devtools/projects/shell-browser/src/assets/BUILD.bazel so it gets brought into the build as well together with the other two font files?

Hi @AleksanderBodurri , a couple of questions from myself:

1. Do I need to run the build and test it in Firefox? The add-on doesn't seem to be ready, or maybe I've missed something.

2. I've updated the MaterialIcons-Regular.ttf file as you will notice in the latest commit because the old file doesn't support some of the icons that are in use (for example: pin_end).
   I firstly used the one from the latest release of material-design-icons (version 4.0 - 3 years ago) which doesn't solve the issue either, so I had to grab the one from its current master branch. I hope it's okay. Link: https://github.com/google/material-design-icons

3. I tried to run the yarn devtools:test but I kept getting this error (which ends up making the testing build failed):
   ERROR: C:/angular_project/angular/devtools/projects/ng-devtools/src/lib/devtools-tabs/profiler/timeline/record-formatter/flamegraph-formatter/BUILD.bazel:39:21: Bundling Javascript devtools/projects/ng-devtools/src/lib/devtools-tabs/profiler/timeline/record-formatter/flamegraph-formatter/test_bundle_spec_entrypoint.mjs [esbuild] failed: (Exit 1): _test_bundle_bundle_esbuild_launcher.bat failed: error executing command bazel-out\x64_windows-opt-exec-2B5CBBC6\bin\devtools\projects\ng-devtools\src\lib\devtools-tabs\profiler\timeline\record-formatter\flamegraph-formatter\_test_bundle_bundle_esbuild_launcher.bat ... (remaining 4 arguments skipped) ERROR: Manifest file C:\Users\Keith\_bazel_Keith\mdlcs3wc\execroot\angular\bazel-out\x64_windows-opt-exec-2B5CBBC6\bin\devtools\projects\ng-devtools\src\lib\devtools-tabs\profiler\timeline\record-formatter\flamegraph-formatter\_test_bundle_bundle_esbuild_launcher.bat.runfiles_manifest does not exist.
   Do you know what's going on here?

Thanks in advance!

Thanks for addressing the comments, I'll answer some of your questions here and do a more thorough review soon.

Hi @AleksanderBodurri , a couple of questions from myself:

1. Do I need to run the build and test it in Firefox? The add-on doesn't seem to be ready, or maybe I've missed something.

The build should be working. Can you elaborate on what you mean by the add-on doesn't seem ready?

2. I've updated the MaterialIcons-Regular.ttf file as you will notice in the latest commit because the old file doesn't support some of the icons that are in use (for example: pin_end).
   I firstly used the one from the latest release of material-design-icons (version 4.0 - 3 years ago) which doesn't solve the issue either, so I had to grab the one from its current master branch. I hope it's okay. Link: https://github.com/google/material-design-icons

This should be fine since it looks like the only code that was relying on the vendored material icons font was removed in #38846. CC @devversion

3. I tried to run the yarn devtools:test but I kept getting this error (which ends up making the testing build failed):
   ERROR: C:/angular_project/angular/devtools/projects/ng-devtools/src/lib/devtools-tabs/profiler/timeline/record-formatter/flamegraph-formatter/BUILD.bazel:39:21: Bundling Javascript devtools/projects/ng-devtools/src/lib/devtools-tabs/profiler/timeline/record-formatter/flamegraph-formatter/test_bundle_spec_entrypoint.mjs [esbuild] failed: (Exit 1): _test_bundle_bundle_esbuild_launcher.bat failed: error executing command bazel-out\x64_windows-opt-exec-2B5CBBC6\bin\devtools\projects\ng-devtools\src\lib\devtools-tabs\profiler\timeline\record-formatter\flamegraph-formatter\_test_bundle_bundle_esbuild_launcher.bat ... (remaining 4 arguments skipped) ERROR: Manifest file C:\Users\Keith\_bazel_Keith\mdlcs3wc\execroot\angular\bazel-out\x64_windows-opt-exec-2B5CBBC6\bin\devtools\projects\ng-devtools\src\lib\devtools-tabs\profiler\timeline\record-formatter\flamegraph-formatter\_test_bundle_bundle_esbuild_launcher.bat.runfiles_manifest does not exist.
   Do you know what's going on here?

Thanks in advance!

Could be some incompatibility with our bazel tests and windows. I'll investigate this more to see what can be done. When this PR leaves draft status the CI will trigger tests that run yarn devtools:test regardless, so you can rely on that in the worst case.

@AleksanderBodurri Thanks for the reply. No, nothing wrong with the firefox add-on, apparently it is some minor issues with my local environment and it's fixed now. I've converted my PR to ready-to-review status, hopefully CI could run the test for me anyhow.

I tried our devtools as a temporary firefox extension and the icons are all loaded in offline mode as well. Please feel free to give it a try and see if it works on your end. Thanks!

@AleksanderBodurri @devversion I have pushed my refactor based on your comments. Do we need to add the "target" label for this pull request to trigger CI testing? Thanks.

cc @mgechev @dylhunn @josephperrott

No CI testing is triggered yet 😕

I see that you just added the action: merge label, but the following checks are still failing:
     status "ci/circleci: test_win" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@hereiskeith Could you rebase this PR onto main? I think that should fix the CI flake. Should be good to merge once thats done

Merge-assistance: I've pushed a rebase to rerun the CI as it was failing the test_win job even after manual CI rerun. The PR is reviewed and approved.

@AleksanderBodurri FYI I've changed the label to target: minor, since this PR is labelled as a feature, so the change can not be merged into the patch branch.

@hereiskeith the rebase triggered a CI rerun and as a result, the lint CI job is now failing due to the commit message on one of the commits, which starts with fixup! fixup!. Could you please merge all commits into one and let us know once it's done (by leaving a comment in this thread), so that we can proceed with next steps? Thank you.

Thanks, @AndrewKushnir for the help and the follow-up. I have merged those three commits into one. Let me know if it's still not working, thanks!

This PR was merged into the repository by commit 5e404d3.