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

[Issue-96] Fix TopBar Widget keeps being visible after Keyboard hides #119

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

ulusoyca
Copy link
Collaborator

@ulusoyca ulusoyca commented Jan 8, 2024

Description

This PR fixes the top bar widget visibility issue due to keyboard closing event (#96) by introducing a new feature in the form of a value notifier, specifically designed to detect the dismissal of the soft-keyboard. This PR primarily impacts two components: WoltModalSheetTopBarFlow and WoltModalSheetTopBarTitleFlow.

Key points

Purpose

The primary goal of this implementation is to trigger a re-paint in the top bar components when the soft-keyboard is closed. This is essential to prevent the top bar from being stuck unresponsive after the keyboard's closing phase.

Default Behavior Sync with Scroll Controller

When isTopBarAlwaysVisible is set to false (by default), the visibility and appearance of the top bar and top bar title are synchronized with the scroll controller's position. Both WoltModalSheetTopBarFlow and WoltModalSheetTopBarTitleFlow use the scroll position data to determine how the top bar should be painted.

Challenge with Keyboard Events

While the appearance of the soft-keyboard naturally triggers scroll update events (courtesy of the Flutter SDK), its dismissal does not inherently cause a similar update in the scroll controller. This discrepancy leads to a situation where the top bar does not repaint as expected when the keyboard closes.

Solution

To address this, the implemented value notifier actively detects the dismissal of the keyboard. Upon this event, it manually triggers a repaint for the top bar flow and top bar title flow widgets. This ensures that the top bar refreshes its state appropriately, maintaining a smooth and responsive user interface.

Additional Changes

  • NotificationController widget inside the WoltModalSheetMainContent is removed since the scroll position is listened directly from the scroll controllers inside WoltModalSheetAnimatedSwitcher widget.
  • Unnecessary passing of the BuildContext to flow delegates is removed.

Screenshots

Before After
fix_before_scroll_position.mp4
fix_after_scroll.mp4

Related Issues

#96

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes tests for all changed/updated/fixed behaviors.
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • The package compiles with the minimum Flutter version stated in the pubspec.yaml

Breaking Change

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

  • Yes, this is a breaking change. The users of this package should set their flutter_keyboard_visibility package version to minimum 6.0.0.
  • [] No, this is not a breaking change.

@ulusoyca ulusoyca requested a review from TahaTesser January 8, 2024 14:20
check only min extent

Repaint after keyboard closing
) async {
if (!visible) {
/// Wait for closing soft keyboard animation to finish before emitting new value.
await Future.delayed(const Duration(milliseconds: 250));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible that different devices will have different keyboard open/close duration which might affect this? I wonder is it possible to read information this from system/device

Copy link
Collaborator

Choose a reason for hiding this comment

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

i've the same concern. This needs to be more testing to be sure.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@TahaTesser how is this problem approached in Flutter in general? I will have a second look to address this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Framework gets native callbacks for events that happen on the system level.

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.

Overall looks good. I wish we didn't have to introduce a package for a such problem but if it works and tested.

@ulusoyca
Copy link
Collaborator Author

ulusoyca commented Jan 9, 2024

Overall looks good. I wish we didn't have to introduce a package for a such problem but if it works and tested.

I spent significant time to resolve this issue without a package but it was really difficult. Thanks for reviewing!

@ulusoyca ulusoyca merged commit b22554c into main Jan 10, 2024
2 checks passed
@ulusoyca ulusoyca deleted the fix-top-bar-visibility-when-keyboard-disappears branch January 10, 2024 14:30
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