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

Remove KeyboardVisibilityListener dependency and add WoltKeyboardClosureListenerMixin #303

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

ulusoyca
Copy link
Collaborator

@ulusoyca ulusoyca commented Aug 14, 2024

Description

This PR introduces the WoltKeyboardClosureListenerMixin mixin and removes the FlutterKeyboardVisibility package dependency. The purpose of this mixin is to track the visibility of the soft keyboard by monitoring the changes in the system's view insets and using the WidgetsBindingObserver.

  • Utilizes the WidgetsBindingObserver to listen to changes in UI metrics, specifically to detect keyboard appearance and disappearance.
  • Updates a ValueNotifier with a new event ID whenever the keyboard closes. This is important for widgets that need to adjust their layout or perform specific actions when the keyboard is hidden. For example, re-painting the top bar and top bar title.
Android iOS
fix_keyboard_visibility.mp4
ios_keyboard.mov

Related Issues

#210

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

@ulusoyca ulusoyca requested a review from TahaTesser August 14, 2024 12:04
@ulusoyca ulusoyca changed the title Remove keyboard visibility listener package and add WoltKeyboardClosureListenerMixin Remove KeyboardVisibilityListener package and add WoltKeyboardClosureListenerMixin Aug 14, 2024
@ulusoyca ulusoyca changed the title Remove KeyboardVisibilityListener package and add WoltKeyboardClosureListenerMixin Remove KeyboardVisibilityListener dependency and add WoltKeyboardClosureListenerMixin Aug 14, 2024
Copy link
Collaborator

@TahaTesser TahaTesser left a comment

Choose a reason for hiding this comment

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

Nicely done! LGTM!

@ulusoyca ulusoyca merged commit cca4790 into main Aug 14, 2024
3 checks passed
@ulusoyca ulusoyca deleted the remove-keyboard-visibility-listener branch August 14, 2024 12:20
Copy link

@moshe5745 moshe5745 left a comment

Choose a reason for hiding this comment

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

Nice work

with TickerProviderStateMixin {
with
TickerProviderStateMixin,
WidgetsBindingObserver,

Choose a reason for hiding this comment

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

Can delete this WidgetsBindingObserver mixin here

/// was previously visible and is now hidden, it increments the event ID and updates
/// the notifier.
@override
void didChangeMetrics() {

Choose a reason for hiding this comment

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

At first i too used this in my first approach.

But then I saw that the MediaQuery is already listening to it.
And its enough just to listen to changes in didChangeDependencies() with MediaQuery.

I don't know what are the implications of this, maybe there arent any.

Copy link
Contributor

@mukireus mukireus left a comment

Choose a reason for hiding this comment

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

This PR is informative and useful. Thanks. 🚀

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.

4 participants