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

Emoji Picker #415

Merged
merged 17 commits into from
Sep 18, 2023
Merged

Emoji Picker #415

merged 17 commits into from
Sep 18, 2023

Conversation

sslater11
Copy link
Contributor

@sslater11 sslater11 commented Sep 15, 2023

  • Added an emoji picker, solves Emoji picker #43
  • Changed the swipes on the number pad key allowing cut copy and paste
  • Added undo/redo, solves Undo / redo buttons #284
  • Updated screenshot
  • Changed most keyboards to use the new emoji key, replacing settings key. See below for issues.

There's a single git commit for incrementing the indentation by 4 spaces, because I have put an if/else clause in KeyboardScreen.kt. The indentation messed up the git diff and it was hard to see what code I had changed, so I fixed the indentation as a separate commit.
It's worth mentioning, because new PRs might have issues if they're adding commits to the old KeyboardScreen.kt file.

For undo/redo, I tried to make an object KeyActions.Undo, and put the code there, but it just doesn't work, so I have done it like this in the CommonKeys.kt file
action = KeyAction.SendEvent( KeyEvent(/* downTime= */ 0, /* eventTime= */ 0, KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_Z, /* repeat= */ 0, /* metaState= */ KeyEvent.META_CTRL_ON)

In KeyboardScreen.kt I have duplicate code for the massive function call KeyboardKey(). I couldn't figure out a better way to do it.

Broken Keyboards:
Keyboards that don't work with the emoji key, and so may be missing the ability to change layouts, because I removed that option from the number pad key and put it on the emoji key.
I don't use these layouts, so I'm unsure what the best way to add emoji to them. We could add a swipe option, or we could add the emoji key. Any recommendations? I would love some input from someone :).

T9v1.kt // Works fine, just missing the emoji key.
TypeSplitDEv1.kt
TypeSplitENv2.kt

TypeSplitESv1.kt
TypeSplitFRv1.kt
TypeSplitITv1.kt
TypeSplitNumeric.kt
TypeSplitPLv1.kt
TypeSplitPTv1.kt
TypesSplitFIv1.kt
WideLayoutENProgrammer.kt

Copy link
Owner

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

This works great! Thanks a ton for this. Just a few fixes / suggestions.

backgroundColor = ColorVariant.SURFACE_VARIANT,
)

val controllerKeys = listOf(emojiBackKey, NUMERIC_KEY_ITEM, RETURN_KEY_ITEM, BACKSPACE_KEY_ITEM)
Copy link
Owner

Choose a reason for hiding this comment

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

The order of the return and backspace need to be switched.

@dessalines
Copy link
Owner

In KeyboardScreen.kt I have duplicate code for the massive function call KeyboardKey(). I couldn't figure out a better way to do it.

I don't think there is... it looks verbose, but there's no way around passing those required params.

I don't use these layouts, so I'm unsure what the best way to add emoji to them. We could add a swipe option, or we could add the emoji key. Any recommendations? I would love some input from someone :).

That's up to those maintainers to upkeep, not you or me. Just replacing the settings key with the new emoji one should be enough for this, and if they'd like to fix / remove some things, they can do so at a later time.

@sslater11
Copy link
Contributor Author

There we go, I've done all the changes you mentioned, although I kept the logging line, because there's one for the numeric mode and shift mode. We should either keep all of them or remove all of them.
I've not worked on moving the duplicate long function call KeyboardKey() to another function, but as you said, that can be done at a later date, so I can look into it :).

Copy link
Owner

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

I just tested and it works great, in dark mode also. Thanks a ton for this, ppl are going to really love it.

@dessalines dessalines merged commit 7ec21c0 into dessalines:main Sep 18, 2023
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.

2 participants