Skip to content

Add Compose support (emoji-compose) - #1258

Draft
DenserMeerkat wants to merge 4 commits into
vanniktech:masterfrom
DenserMeerkat:emoji-compose
Draft

Add Compose support (emoji-compose)#1258
DenserMeerkat wants to merge 4 commits into
vanniktech:masterfrom
DenserMeerkat:emoji-compose

Conversation

@DenserMeerkat

Copy link
Copy Markdown

This PR introduces the new emoji-compose module to bring Jetpack / Compose Multiplatform support to the library.
resolves #690

What's included

  • New emoji-compose artifact containing @Composable components:
    • EmojiPicker: Category tab grid picker with recent & variant support.
    • EmojiImage: Image painter/composable for rendering provider graphics.
    • EmojiPickerVariantPopup: Popup for selecting variants.
    • EmojiPickerDefaults: Theming and color configurations.

Currently, this focuses on the Android target. Will add more platforms and a sample compose app.

@vanniktech vanniktech left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

First of all sorry for not reviewing this earlier! Second please bear with me, it's been a while that I've been inside this library

  • can we create a new sample app on how to use it? like sample-compose
  • this really needs to be multiplatform, with the sample app also being able to run on ios/android & jvm (desktop)

/**
* Renders the category tab icon using Android vector resources or a text emoji fallback.
*/
@Composable actual fun CategoryTabIcon(

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

why is this in androidMain? I don't see anything that should not be avaiable from commonMain, the resource for the icon we can also put there!

* Returns the default [RecentEmojiManager] for Android using [LocalContext].
*/
@Composable actual fun rememberDefaultRecentEmoji(): RecentEmoji {
val context = LocalContext.current

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There must also be something for commonMain, right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

For commonMain, should we default to NoRecentEmoji / NoVariantEmoji or something like an in-memory session manager? And for other platforms native storage like NSUserDefaults for iOS and Preferences for Desktop?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

NSUserDefaults for iOS, Preferences for Desktop and then we don't need the NoOp, right?

val drawable = remember(emoji, provider, context) {
if (provider is EmojiAndroidProvider) {
try {
provider.getDrawable(emoji, context)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hmmm can we use skia or the normal resource system to make this truly multiplatform?

* Returns the preferred Unicode representation of this emoji, preferring Variation Selector 16
* (\uFE0F) to ensure full-color graphical presentation.
*/
fun Emoji.preferredUnicode(): String {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't we have this logic already somewhere, if not, why do we need it?

* Filters and returns the list of skin tone variants for the given [emoji].
* Excludes non-variant selector entries (`isVariantSelector16`) and base duplicates.
*/
fun filterMeaningfulVariants(emoji: Emoji, variantEmoji: VariantEmoji? = null): List<Emoji> {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't we have this logic already somewhere, if not, why do we need it?

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.

Integration with Jetpack Compose ?

2 participants