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

Fix twemoji cdn + add twemoji assets to public #1070

Closed
wants to merge 5 commits into from

Conversation

Airyzz
Copy link

@Airyzz Airyzz commented Jan 11, 2023

Description

Due to the twemoji cdn outage, we need to use a different cdn. I am also taking this opportunity to include the twemoji assets in the app itself, for use in cinny-desktop.

This adds twitter/twemoji as a submodule in /deps/twemoji

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Now only the twemoji resources are copied to public at build time, that way we arent shipping any of the twemoji source as well
@Airyzz Airyzz changed the title Fix twemoji cdn + add local storage Fix twemoji cdn + add twemoji assets to public Jan 11, 2023
@github-actions
Copy link

github-actions bot commented Jan 13, 2023

Preview: https://1070--pr-cinny.netlify.app
⚠️ Exercise caution. Use test accounts. ⚠️

src/app/organisms/emoji-board/EmojiBoard.jsx Outdated Show resolved Hide resolved
src/util/twemojify.jsx Outdated Show resolved Hide resolved
src/util/twemojify.jsx Outdated Show resolved Hide resolved
src/util/twemojify.jsx Outdated Show resolved Hide resolved
src/util/twemojify.jsx Outdated Show resolved Hide resolved
@Airyzz Airyzz marked this pull request as ready for review January 13, 2023 07:36
@Airyzz Airyzz requested a review from ajbura January 13, 2023 07:36
@kfiven
Copy link
Collaborator

kfiven commented Jan 13, 2023

To work this correctly

- name: Checkout repository
        uses: actions/checkout@v3.2.0

needs to be replaced by

- name: Checkout repository
        uses: actions/checkout@v3.2.0
        with: 
          submodules: true

in GitHub actions files.

@ajbura
Copy link
Member

ajbura commented Jan 14, 2023

We got build time and bundle size increased with this way. I think we should use a svg sprite maybe with (https://www.npmjs.com/package/@svgmoji/twemoji) or something else?

I have done a temp fix in e5e3f5f until we merge this.

@Airyzz
Copy link
Author

Airyzz commented Jan 14, 2023

We got build time and bundle size increased with this way. I think we should use a svg sprite maybe with (https://www.npmjs.com/package/@svgmoji/twemoji) or something else?

I have done a temp fix in e5e3f5f until we merge this.

I can change to only copy the svg assets in at build time and use those instead, but looking at file sizes, the svg are actually larger than the 72x72 emoji.

All SVG: 8.6MB
All PNG: 3.3MB

is the bundle size / build time so significantly larger that it's a blocker?

@ajbura
Copy link
Member

ajbura commented Jan 23, 2023

@Airyzz Let's just use the mozilla/twemoji-colr font. As discussed in room it won't require crazy setup and will improve the overall DX.

@Airyzz
Copy link
Author

Airyzz commented Jan 23, 2023

@Airyzz Let's just use the mozilla/twemoji-colr font. As discussed in room it won't require crazy setup and will improve the overall DX.

OK, what should we do for platforms which dont support this font? Fallback to CDN? or just not deal with it for now?

@ajbura
Copy link
Member

ajbura commented Jan 24, 2023

95% user can render it for other they will get rendered as os default emoji's

@kfiven
Copy link
Collaborator

kfiven commented May 22, 2024

Closing as twemoji has been added as fonts.

@kfiven kfiven closed this May 22, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants