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

feat: add animated emoji support #450

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheOneWithTheBraid
Copy link
Collaborator

  • implement animated emoji support in both HTML and Linkify message type
  • fix some missing font glyphs
  • trim message input

@TheOneWithTheBraid TheOneWithTheBraid force-pushed the braid/animated-emoji-github branch 2 times, most recently from 51bae14 to 65b1628 Compare July 21, 2023 06:19
@krille-chan
Copy link
Owner

Can you clean up the commit history and make sure that every commit does only one thing? For example why do you change style: const TextStyle(fontFamily: 'RobotoMono'), to style: const TextStyle(fontFamily: 'Roboto Mono'),? Does it fix something? There is nothing in the commit message about it and what has this to do with animated emoji support?

@TheOneWithTheBraid
Copy link
Collaborator Author

Can you clean up the commit history and make sure that every commit does only one thing? For example why do you change style: const TextStyle(fontFamily: 'RobotoMono'), to style: const TextStyle(fontFamily: 'Roboto Mono'),? Does it fix something? There is nothing in the commit message about it and what has this to do with animated emoji support?

Yeah, sorry, the commit history completely broke from rebasing and migrating onto the new upsream. These commits fix what's stated in the pubspec.yaml, which is at least about the noto font directly related to emojis. Since it'd feel weird to only fix it for one of the two font families, I fixedit for roboto too - though it's actually not related to the emoji support.

Sorry again, the commit history was re-written many times, since also the upstream commit history was re-written and during cherry-picking in the commits onto the new upstream history, most of the commit messages were somehow breaking again.

If you want, I can take the time to restore them, but that'd be quite lots of manual work.

@TheOneWithTheBraid
Copy link
Collaborator Author

  • would therefore rather prefer to squash the messy history

@TheOneWithTheBraid
Copy link
Collaborator Author

Poke @krille-chan : Do you have any update on this Pull Request ?

@krille-chan
Copy link
Owner

Poke @krille-chan : Do you have any update on this Pull Request ?

As long as it does bring so much more stuff to load for the initial start of the web app I unfortunately see no way to merge this. :-/ It could be possible by not shipping this in the web-version if it works to fallback to normal emojis there

@TheOneWithTheBraid
Copy link
Collaborator Author

Poke @krille-chan : Do you have any update on this Pull Request ?

As long as it does bring so much more stuff to load for the initial start of the web app I unfortunately see no way to merge this. :-/ It could be possible by not shipping this in the web-version if it works to fallback to normal emojis there

What do you mean by this ? There are no new assets added or changed. Do you have concerns about loading the dart_animated_emoji package ? I could simply deffer it and only load it on demand. Would you prefer that ?

- implement animated emoji support in both HTML and Linkify message type
- fix some missing font glyphs
- trim message input

Signed-off-by: The one with the braid <[email protected]>
@MagicRB
Copy link

MagicRB commented May 5, 2024

Any updates on this? Animated emojis would be nice

@krille-chan
Copy link
Owner

Any updates on this? Animated emojis would be nice

just had no time for this yet. My concerns are that this delays the start-up on web for too long or loads too much from google servers (while the web app already loads emojis from google servers unfortunately)

@TheOneWithTheBraid
Copy link
Collaborator Author

Yes, that was exactly the issue. Basically we'd need to decide : Do we want super slow startup and no stuff from google servers (initial load +5MB), slow startup and only fallback glyphs from google servers (+0.3MB startup) or everything from Google servers and fast startup. Either options seems like a regression. Though I actually removed all handling of that case into the #451 so that we don't need to find a solution for this issue before merging this.

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.

None yet

3 participants