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

refactor(neon_framework): Implement a custom emoji picker #2623

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

provokateurin
Copy link
Member

Fixes #2499

I thought implementing a custom emoji picker would be hard because all the existing packages suck and are unmaintained, but it turned out it was quite simple in the end.

Because the available emojis are generated from the metadata they are also more complete than what we had before, because the package was manually adding them to a list.

This picker does not support alternates (e.g. skin tones) yet, but it can be added easily.
It also does not save any recently used emojis, but we can add that as well.

Due to having all the metadata available we can now also add shortcode (e.g. :smile:) completion for text inputs (e.g. Talk message input) which is pretty neat.

For some reason the Tabbar renders a bit weird on Android and I believe it might be connected to flutter/flutter#119623 as I was seeing that issue as well. On Linux it renders perfectly and the emojis are centered as well 🤷‍♀️

@provokateurin provokateurin force-pushed the refactor/neon_framework/custom-emoji-picker branch from 4095aa3 to 6c8177f Compare November 1, 2024 14:18
Copy link

codecov bot commented Nov 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.90%. Comparing base (b389406) to head (8317e47).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2623   +/-   ##
=======================================
  Coverage   28.90%   28.90%           
=======================================
  Files         371      372    +1     
  Lines      136623   136624    +1     
=======================================
+ Hits        39491    39492    +1     
  Misses      97132    97132           
Flag Coverage Δ *Carryforward flag
account_repository 98.47% <ø> (ø)
cookie_store 99.48% <ø> (ø) Carriedforward from b389406
dashboard_app 96.05% <ø> (ø)
dynamite 31.05% <ø> (ø) Carriedforward from b389406
dynamite_end_to_end_test 61.69% <ø> (ø) Carriedforward from b389406
dynamite_runtime 85.40% <ø> (ø) Carriedforward from b389406
interceptor_http_client 97.18% <ø> (ø) Carriedforward from b389406
neon_dashboard 96.05% <ø> (ø) Carriedforward from b389406
neon_framework 61.44% <ø> (+0.01%) ⬆️
neon_http_client 94.32% <ø> (ø) Carriedforward from b389406
neon_notifications 100.00% <ø> (ø) Carriedforward from b389406
neon_storage 94.66% <ø> (ø) Carriedforward from b389406
neon_talk 99.45% <ø> (ø) Carriedforward from b389406
nextcloud 24.33% <ø> (ø) Carriedforward from b389406
notifications_app 97.43% <ø> (ø)
notifications_push_repository 98.11% <ø> (ø)
sort_box 90.90% <ø> (ø) Carriedforward from b389406
talk_app 99.01% <100.00%> (ø)

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
packages/neon_framework/lib/src/utils/emojis.dart 100.00% <ø> (ø)
...ckages/talk_app/lib/src/widgets/message_input.dart 98.56% <100.00%> (ø)
---- 🚨 Try these New Features:

@Leptopoda
Copy link
Member

Really cool. I didn't take a look at the specific code yet but I have a few ideas:

  1. what about making this a separate package. Might be easier to maintain and this package could then also handle utility functions for the :name: search.
  2. Why not use the officil unicode cldr? That way we could already offer support for all lamguages:
    https://github.com/unicode-org/cldr-json/blob/main/cldr-json%2Fcldr-annotations-full%2Fannotations%2Fde%2Fannotations.json (alternate emojis like skin tone are im a separate file in the same repo)

I'll take a deeper look at thw UI part later

@provokateurin
Copy link
Member Author

what about making this a separate package

I thought about this as well, but right now the code is still quite simple.
Would you just try to separate the code or also publish the package at some point?
There are already a lot of emoji picker packages out there, so naming would be a problem.

Why not use the officil unicode cldr

For what? I don't understand where you would want to use it.

@Leptopoda
Copy link
Member

For what? I don't understand where you would want to use it.

I assume that the emoji category names are currently hand translated.
The unicode cldr already contains official translations for all languages supported by unicode.
These could be used to also generate arb files for all languages making the emoji picker more complete.

Would you just try to separate the code or also publish the package at some point?

Once we have translations and search strings for all emojis/languages the app size might increase quite significantly.
Because the emoji picker will not be used by all apps I'd rather have separate arb files for them that are only inserted into the widget tree for the widgets in question.

Even if you we keep it in the neon_framework package, I'd like to have this separation.

@provokateurin
Copy link
Member Author

provokateurin commented Nov 2, 2024

I assume that the emoji category names are currently hand translated.

Yes

The unicode cldr already contains official translations for all languages supported by unicode.

It contains translations for all the symbols, but not for the category names as they are not unicode symbols themselves.

Once we have translations and search strings for all emojis/languages the app size might increase quite significantly.

Don't forget it's all just string data.
Also we don't need any translations for the emojis, as translation software and screenreaders do this already internally (they probably use the CLDR data themselves).
The only translations we need are the group names and there are not many so it's insignificant.

I just did a size analysis of the APK and emojis.dart is 0.1KB and the NeonEmojiPickerDialog is 39.5KB.
I think this is totally acceptable and we have other places that should be optimized first before doing anything here.
I also did a comparison of the current main branch and this branch and the overall size shrunk by 0.6KB (less than I expected, but it's not more so a win nevertheless).

@provokateurin
Copy link
Member Author

We might want to ship and use an Emoji font like https://github.com/googlefonts/noto-emoji directly, to ensure all platforms render them correctly (e.g. different Android versions support different Unicode standards) and the same way.
I think only on iOS and macOS it could go "wrong", because they normally use a different font and not Noto Emoji.

@Leptopoda
Copy link
Member

I don't know if it is worth the apk size bump.
Maybe we can use the appcompat lib on android:
https://developer.android.com/develop/ui/views/text-and-emoji/emoji2

It's supported since android 11 (~80% of active devices).

@provokateurin
Copy link
Member Author

provokateurin commented Nov 16, 2024

Ah nice I didn't know about that. Unfortunately it says this:

Your app automatically displays backward-compatible emoji on all devices that provide an emoji2-compatible downloadable fonts provider, such as devices powered by Google Play services.

And my guess would be that it won't work on devices without Google Play services (I would have to test it though).

Reading through the page I'm not sure if it is also going to work with flutter, given that it doesn't use the native font rendering. I guess we just need to give it a try to see if it works (also see flutter/flutter#16660).

@provokateurin
Copy link
Member Author

I don't know if it is worth the apk size bump.

Hm yeah it's 10.2 MB, that is 1/3 of our current APK size and it doesn't give us that much benefits except for less broken emojis.

Maybe the better way to deal with this is to dynamically decide at runtime which emojis to keep, so checking the Android API level and then deciding which Unicode version we can assume to be available.

@provokateurin
Copy link
Member Author

We could also use https://pub.dev/packages/google_fonts and have the emoji font downloaded at runtime, but there we run into other issues again as we have to explicitly use the font everywhere (see material-foundation/flutter-packages#155)

@provokateurin
Copy link
Member Author

Found the CLDR data for the emoji groups: https://github.com/unicode-org/cldr-json/blob/main/cldr-json/cldr-misc-full/main/de/characterLabels.json.
Let's use them in the future and keep the manual translations for now.

packages/neon_framework/lib/src/utils/emojis.dart Outdated Show resolved Hide resolved
packages/neon_framework/generate_emojis.dart Outdated Show resolved Hide resolved
packages/neon_framework/generate_emojis.dart Outdated Show resolved Hide resolved
@provokateurin provokateurin force-pushed the refactor/neon_framework/custom-emoji-picker branch from 6c8177f to 6919ea6 Compare November 19, 2024 19:58
@provokateurin provokateurin force-pushed the refactor/neon_framework/custom-emoji-picker branch from 6919ea6 to 203b249 Compare November 19, 2024 20:37
.cspell/misc.txt Outdated Show resolved Hide resolved
@provokateurin provokateurin force-pushed the refactor/neon_framework/custom-emoji-picker branch from 203b249 to 8317e47 Compare November 20, 2024 08:13
@provokateurin provokateurin merged commit a26468d into main Nov 20, 2024
11 checks passed
@provokateurin provokateurin deleted the refactor/neon_framework/custom-emoji-picker branch November 20, 2024 08:29
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.

Emoji picker looks bad on smaller screens
2 participants