Skip to content

fix(flag): use jdecked twemoji fork#3169

Merged
lubber-de merged 3 commits intofomantic:developfrom
mvorisek:fix_twemoji_upgrade
Feb 8, 2025
Merged

fix(flag): use jdecked twemoji fork#3169
lubber-de merged 3 commits intofomantic:developfrom
mvorisek:fix_twemoji_upgrade

Conversation

@mvorisek
Copy link
Copy Markdown
Contributor

@mvorisek mvorisek commented Feb 8, 2025

I belive this was missed in #3086.

@lubber-de
Copy link
Copy Markdown
Member

lubber-de commented Feb 8, 2025

This was basically untouched intentionally

  • Flag did not receive a change/update in 15.1 (no flags updated/added)
  • twitter theme is still meant for the original twitter (14) emojis. If we just change the url, the v15 emojis still aren't available as the new emojis are not listed inside the twitter/emoji.variables file

So:

  • The extra twitter theme (for emoji) wouldn't make sense anymore (perhaps never was) if it's always the same as the default themes emojis, we might either drop it (breaking change?) or leave it as it is, so it remains the "real" twitter theme
  • For flag, we could/should change the repo url to at least share the same source as the default emojis

Copy link
Copy Markdown
Member

@lubber-de lubber-de left a comment

Choose a reason for hiding this comment

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

LGTM

@mvorisek
Copy link
Copy Markdown
Contributor Author

mvorisek commented Feb 8, 2025

Twitter theme - flags can this be used there, so I would say it should be upgraded as well for unified UX as long as no flag was removed. But I reverted this change.

Flags - this should definitely be upgraded. And there is actually change in one flag 😀:
image

before:
image

after:
image

(height has changed)

The reason for this PR is: https://github.com/atk4/ui/blob/5.2.0/public/external/postinstall.js#L151 - in atk4/ui we bundle all the flags so the atk4/ui can work fully offline.

@lubber-de lubber-de changed the title Use "jdecked/twemoji" sources everywhere fix(flag): use jdecked twemoji fork Feb 8, 2025
@lubber-de lubber-de merged commit 96ed453 into fomantic:develop Feb 8, 2025
@lubber-de lubber-de added this to the 2.9.4 milestone Feb 8, 2025
@lubber-de lubber-de added the type/bug Any issue which is a bug or PR which fixes a bug label Feb 8, 2025
@mvorisek mvorisek deleted the fix_twemoji_upgrade branch February 8, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Any issue which is a bug or PR which fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants