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

iOS fix keyboard crash #1383

Merged
merged 7 commits into from
Jun 5, 2024
Merged

iOS fix keyboard crash #1383

merged 7 commits into from
Jun 5, 2024

Conversation

elijah-semyonov
Copy link

Properly handle UIPress with key == nil

Fixes a crash JetBrains/compose-multiplatform#4911

Testing

The reported issue doesn't reproduce anymore.

Release Notes

Fixes - iOS

  • Pressing directional keys on a physical keyboard connected to iOS device doesn't cause a crash.

}

val key = specialTypeKey ?: pressKey?.keyCode?.let { Key(it) } ?: Key.Unknown
val codePoint = pressKey?.characters?.firstOrNull()?.code ?: 0
Copy link
Member

Choose a reason for hiding this comment

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

Please reuse our internal helper String.codePointAt(index: Int): CodePoint instead of firstOrNull

Copy link
Author

@elijah-semyonov elijah-semyonov Jun 4, 2024

Choose a reason for hiding this comment

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

I feel hesitant to introduce this change in this PR:

  1. I'm not owning this piece of code.
  2. This is out of the scope of the fix.
  3. So far we have no issues related to it.

Copy link
Member

Choose a reason for hiding this comment

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

Let's add explicit TODO at least

Copy link
Author

Choose a reason for hiding this comment

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

Done

@elijah-semyonov elijah-semyonov changed the title Es/fix keyboard crash iOS fix keyboard crash Jun 4, 2024
…ey/KeyEvent.uikit.kt

Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>
Copy link
Collaborator

@Schahen Schahen left a comment

Choose a reason for hiding this comment

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

Apart from what Ivan wrote about don't see anything unacceptable )

// can be potentially nil on TVOS, this will cause a crash
val uiKey = requireNotNull(key) {
"UIPress with null key is not supported"
val pressKey = key
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, if we don't need any additional check then (why btw?) then this variable is redundant and we can just use key

Copy link
Author

Choose a reason for hiding this comment

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

Aha, removed extra variable and avoided shadowing this.key with local key.

…ey/KeyEvent.uikit.kt

Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>
@elijah-semyonov elijah-semyonov merged commit 6ad8736 into jb-main Jun 5, 2024
6 checks passed
@elijah-semyonov elijah-semyonov deleted the es/fix-keyboard-crash branch June 5, 2024 06:49
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.

3 participants