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

Replace fontawesome cdn by react-fontawesome #706

Merged
merged 6 commits into from
Feb 20, 2024

Conversation

vincentfretin
Copy link
Contributor

@vincentfretin vincentfretin commented Feb 4, 2024

This closes #692
Some icons slightly changed.

Remaining to do:

  • Regression for the collapsible panel when clicking on clipboard icon
  • save icon
  • icons for types camera: 'fa-camera', mesh: 'fa-cube', light: 'fa-lightbulb-o', text: 'fa-font'

@vincentfretin
Copy link
Contributor Author

Before:
Capture d’écran du 2024-02-18 18-35-52

After:
Capture d’écran du 2024-02-18 19-45-51

@vincentfretin vincentfretin marked this pull request as ready for review February 18, 2024 18:48
@vincentfretin
Copy link
Contributor Author

vincentfretin commented Feb 19, 2024

I replaced clipboard by clipbard-copy mainly because I couldn't figure out how to stop the click event propagating and collapse the panel. Switching to clipboard-copy reduce a bit the bundle size:
https://bundlephobia.com/package/clipboard@2.0.11 9kB minified
https://bundlephobia.com/package/clipboard-copy@4.0.1 838B minified

We removed downloading https://maxcdn.bootstrapcdn.com/font-awesome/4.5.0/css/font-awesome.min.css here, so 27K less to download as well.

Before this PR:
483K aframe-inspector.min.js
After this PR:
551K aframe-inspector.min.js

increasing bundle to 68kB, wait this doesn't line up, I would had expected 490K maybe.
I confirm bundling without clipboard-copy is 550K.
So it's just the FontAwesomeIcon component that is 60K? That's crazy.
I'll investigate further later, I'll probably create a simple Icon React component just to render the svg. I put this PR back in draft.

Also just to confirm this wasn't due to bad tree shaking, I replaced

import {
  faCamera,
  faCube,
  faFont,
  faLightbulb
} from '@fortawesome/free-solid-svg-icons';

to

import { faCamera } from '@fortawesome/free-solid-svg-icons/faCamera';
import { faCube } from '@fortawesome/free-solid-svg-icons/faCube';
import { faFont } from '@fortawesome/free-solid-svg-icons/faFont';
import { faLightbulb } from '@fortawesome/free-solid-svg-icons/faLightbulb';

the bundle increased by 3KB TT.

@vincentfretin vincentfretin marked this pull request as draft February 19, 2024 09:33
@dmarcos dmarcos marked this pull request as ready for review February 20, 2024 17:03
@dmarcos dmarcos merged commit 5d75aae into aframevr:master Feb 20, 2024
1 check passed
@dmarcos
Copy link
Member

dmarcos commented Feb 20, 2024

Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use icons from @fortawesome/react-fontawesome
2 participants