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

[GH-8636] Prevent Cross-fade out keyboard to fill up view #8638

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

anlerandy
Copy link

Summary

Prevent keyboard avoiding space to fill up the view due to useAnimatedKeyboard incorrect value.

Both state & height returned by useAnimatedKeyboard get corrupted after keyboard close if "Cross-fade transitions" is active - Height is approximately equal to view height ; State is put back to "OPEN".

Also, bug can occur using external keyboard ; Keyboard will open briefly before closing.

Ticket Link

#8636

Device Information

This PR was tested on:

  • iOS 18.3.1 - iPhone 11 Pro Max
  • iOS 18.1.1 - iPhone 15 (Simulator)
  • Android 15 - Medium Phone API 35 (Emulator)

Release Note

Fix Cross-Fade keyboard failed to close

@mm-cloud-bot mm-cloud-bot added the kind/bug Categorizes issue or PR as related to a bug. label Feb 27, 2025
@mattermost-build
Copy link
Contributor

Hello @anlerandy,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?
Once you have signed the CLA, please comment with /check-cla and confirm that the CLA check is green.

This is a standard procedure for many open source projects.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

@rahimrahman rahimrahman requested review from Copilot, enahum and a team and removed request for a team February 28, 2025 13:19

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lieut-data lieut-data removed the request for review from a team February 28, 2025 14:59
@Willyfrog
Copy link
Contributor

Hi @anlerandy , thanks for the PR!!

can you sign the CLA? otherwise we can't accept any changes. If you have already done so, let us know to check again or see if there is any problem with the automation.

@Willyfrog Willyfrog self-requested a review March 3, 2025 11:32
@Willyfrog Willyfrog assigned Willyfrog and anlerandy and unassigned Willyfrog Mar 3, 2025
@anlerandy
Copy link
Author

anlerandy commented Mar 3, 2025

Hi @Willyfrog,

I do have a problem with CLA. I've used /check-cla without knowing if that's the solution.

@Willyfrog
Copy link
Contributor

I think I know what's going on, I'll need to ping some people in the company but this week there is a company wide event and will take some time.

@Willyfrog
Copy link
Contributor

@anlerandy hopefully everything is fixed as of now, but it is likely your signature was missing, can you sing again the CLA?

you'll find the form at http://www.mattermost.org/mattermost-contributor-agreement/

@anlerandy
Copy link
Author

Thanks, @Willyfrog!
I'm in the CLA. I didn't do anything so I guess my previous attempts worked.

Copy link
Contributor

@Willyfrog Willyfrog left a comment

Choose a reason for hiding this comment

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

left a question, but otherwise looks ok

};
}, [context, insets.bottom, offset]);
}, [context, insets.bottom, offset, isNativeKeyOpen]);
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't keyb or keyb.state be a dependency?

Copy link
Author

Choose a reason for hiding this comment

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

While testing, adding it to dependencies didn't change much things, at least not anything related to this PR's fix.
So I decided to leave code as close as it was.

Copy link
Contributor

@enahum enahum left a comment

Choose a reason for hiding this comment

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

this has been fixed in reanimated 3.17.1, I guess the best solution would be to upgrade the dependency.

software-mansion/react-native-reanimated#6440

Copy link
Contributor

@enahum enahum left a comment

Choose a reason for hiding this comment

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

One last thing remaining is to rename the patch for reanimated in the patches folder

@anlerandy
Copy link
Author

Thanks, @enahum, for pointing out the update. It worked perfectly with just the update.

Could you take another look?

@anlerandy anlerandy requested a review from enahum March 6, 2025 12:41
@enahum enahum added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester and removed Awaiting Submitter Action null labels Mar 6, 2025
@enahum enahum requested a review from yasserfaraazkhan March 6, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester Contributor kind/bug Categorizes issue or PR as related to a bug. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants