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] Support keyboard animation on iOS #29281

Merged
merged 25 commits into from
Nov 8, 2021

Conversation

luckysmg
Copy link
Contributor

@luckysmg luckysmg commented Oct 21, 2021

This PR supports FlutterViewController switch keyboard with smooth animation

Before:

no_animation.MP4

What I have done:

with_animation.MP4

List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#19279
flutter/flutter#92248

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@nikitadol
Copy link

@yasinarik
Copy link

yasinarik commented Oct 21, 2021

@luckysmg Thank you for putting in all this effort. I hope it won't take too much time for this PR to be merged. Congrats.

According to the video, it animates and pushes the available area smoothly.

Please forgive me if these are already included but I want to emphasize two important features :

  1. Let custom input for the animation duration and curve
  2. Allow keyboard dragging first and decide to open or close the keyboard after the drag ends. For example, WhatsApp and Telegram apps are allowing such behavior. The keyboard moves according to the touch drag so it can have mid offset values. It will be more native.

@luckysmg
Copy link
Contributor Author

Yes.The keyboard animation has a curve with rawValue 7
But I found that the animation still sync with keyboard animation when I didn't set the animation curve.
I think Apple did something in iOS SDK to sync this animation curve implicitly @nikitadol

@nikitadol
Copy link

Yes.The keyboard animation has a curve with rawValue 7 But I found that the animation still sync with keyboard animation when I didn't set the animation curve. I think Apple did something in iOS SDK to sync this animation curve implicitly @nikitadol

It looks like it's a coincidence. I think it is better to use curves if there are any (this is just a couple of lines of code)

@luckysmg
Copy link
Contributor Author

luckysmg commented Oct 22, 2021 via email

@luckysmg
Copy link
Contributor Author

luckysmg commented Oct 22, 2021

I know,but I don't know how use this in UIViewAnimationOptions in

 [UIView animateWithDuration:(NSTimeInterval) delay:(NSTimeInterval) options:(UIViewAnimationOptions) animations:^{
            
        } completion:^(BOOL finished) {
            
        }]

Because the keyboard animation curve is not a public value with 7.So I'm not sure how to write it.
Could you give me a code ? Thx😄 @nikitadol

@nikitadol
Copy link

I know,but I don't know how use this in UIViewAnimationOptions in

 [UIView animateWithDuration:(NSTimeInterval) delay:(NSTimeInterval) options:(UIViewAnimationOptions) animations:^{
            
        } completion:^(BOOL finished) {
            
        }]

Because the keyboard animation curve is not a public value with 7.So I'm not sure how to write it. Could you give me a code ? Thx😄 @nikitadol

For iOS >= 10

        let curve = UIView.AnimationCurve(rawValue: curveValue)!

        let animator = UIViewPropertyAnimator(
            duration: duration,
            curve: curve
        ) {
            self.keyboardAnimationView.frame = CGRect(0, insetBottomEndValue, 0, 0)
        }
        
        animator.startAnimation()
        
        animator.addCompletion { finalPosition in
            if (finalPosition == .end) {
                self.keyboardAnimationLink.invalidate()
                self.keyboardAnimationLink = nil
            }
        }

@luckysmg
Copy link
Contributor Author

Got it , I will try it. Thx

@luckysmg luckysmg changed the title Support keyboard animation on iOS [iOS] Support keyboard animation on iOS Oct 25, 2021
@luckysmg
Copy link
Contributor Author

luckysmg commented Oct 26, 2021

I have tested the new animation api.If use new API,the code will be a lot.
So,If this plan is available(refresh with displaylink)and flutter team adopt this .I will add new Animation API in this code if necessary @nikitadol.😄

@nikitadol
Copy link

You are using setAnimationCurve which is deprecated

If Apple decides to remove it in iOS 16, then no one can build the Flutter app until this code is updated

And if it does update, then everyone will have to update the Flutter version (although this can take months or years)

I mean, Apple marked this API out of date 2 years ago for a reason
And it will be very unpleasant to rewrite this code again

@luckysmg
Copy link
Contributor Author

luckysmg commented Oct 26, 2021

Yes,if flutter team agrees the plan in this code (refresh with displaylink and get interpolation in a view),I will add new animation API immediately。I will never use a deprecated API in this PR after flutter team agrees this plan😄

@luckysmg
Copy link
Contributor Author

luckysmg commented Oct 26, 2021

I thought about it..I just use new API directly for review😄...@nikitadol

@Oleh-Sv
Copy link
Contributor

Oleh-Sv commented Nov 7, 2021

@luckysmg it looks really better 👍

Did you try to make flutter view as subview of root view and update constant of constraint with parallel update of viewMetrics in onDisplay method?

Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@joellurcook
Copy link

This PR introduces an issue for SafeArea widgets when maintainBottomViewPadding is set to true - the bottom viewPadding is not respected at all times during the animation.

SafeArea contains the following test:

if (data.padding.bottom == 0.0 && data.viewInsets.bottom != 0.0 && maintainBottomViewPadding)
      padding = padding.copyWith(bottom: data.viewPadding.bottom);

However, it is possible for padding.bottom to be > 0.0 whilst viewInsets.bottom != 0.0 during animation.

I would suggest something more like:

if (data.padding.bottom < data.viewPadding.bottom &&
        data.viewInsets.bottom != 0.0 &&
        maintainBottomViewPadding) 
      padding = padding.copyWith(bottom: data.viewPadding.bottom);

I have not tested this fix extensively, but it appears to work fine for iOS

@gaaclarke
Copy link
Member

Hey @joellurcook , nice detective work. I recommend first filing an issue to describes the problem (with animations a screen recording can help communicate the problem). Then put your proposed change in a PR and link the issue. That's the proper method to get eyeballs on your problem/suggestion.

@nizwar
Copy link

nizwar commented Feb 13, 2022

Hi, i just wonder how to not use this? for me all good and feel smooth, but my client not really like the animation

@luckysmg
Copy link
Contributor Author

luckysmg commented Feb 23, 2022 via email

@yyong37
Copy link

yyong37 commented Feb 23, 2022

@nizwar as my test, wrap with MediaQuery can do

@dhnghia22
Copy link

@luckysmg hello, thanks for your contribute, but i got some problem with this feature, modalBottomSheet and dialog have small animation change position when keyboard appear or dismiss, so same question with @nizwar, anyway to not use this ?

@luckysmg
Copy link
Contributor Author

Hi @dhnghia22 ,maybe this is the same issue like flutter/flutter#97609 in framework side

@aurangzaibsiddiqui
Copy link

Hi @luckysmg

This is a pretty neat update to the iOS Flutter experience.

However, it is causing issues for my chat application. As you can see in the attached video, the smooth keyboard animation is quite visibly janky (even in release mode). This jank was not there when the animation was not smooth (i.e. before you submitted your fix).

Is there some way to address the jank? If not, is it possible to not use the smooth animation at all (like previously).

RPReplay_Final1654775077.MP4

@NghiaDinhShopiness
Copy link

NghiaDinhShopiness commented Jun 26, 2022

Hi @dhnghia22 ,maybe this is the same issue like flutter/flutter#97609 in framework side

I still got this problem when upgrade to flutter 3.0.3

RPReplay_Final1656216934.MP4

On video below, AlertDialog move up when keyboard appear, and dialog have unexpected animation when display when keyboard dismiss. Anyway to fix it.

import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: MyHomePage(),
    );
  }
}

class MyHomePage extends StatelessWidget {
  const MyHomePage({Key? key}) : super(key: key);

  showAlertDialog(BuildContext context) {
    // set up the button
    Widget okButton = TextButton(
      child: Text("OK"),
      onPressed: () {
        Navigator.of(context).pop();
      },
    );

    // set up the AlertDialog
    AlertDialog alert = AlertDialog(
      title: Text("My title"),
      content: Text("This is my message."),
      actions: [
        okButton,
      ],
    );

    // show the dialog
    showDialog(
      context: context,
      builder: (BuildContext context) {
        return alert;
      },
    );
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      resizeToAvoidBottomInset: false,
      appBar: AppBar(
        title: const Text('Display Alert with keyboard'),
      ),
      body: Center(
        child: Column(
          children: <Widget>[
            Container(
              margin: EdgeInsets.only(top: 50),
              child: Column(
                children: [
                  TextButton(
                      onPressed: () {
                        FocusManager.instance.primaryFocus?.unfocus();
                        showAlertDialog(context);
                      },
                      child: Text('Button')),
                  SizedBox(width: 200, child: TextField())
                ],
              ),
            ),
          ],
        ),
      ),
    );
  }
}

flutter --version

Flutter 3.0.3 • channel unknown • unknown source
Framework • revision 676cefaaff (3 days ago) • 2022-06-22 11:34:49 -0700
Engine • revision ffe7b86a1e
Tools • Dart 2.17.5 • DevTools 2.12.2

@luckysmg
Copy link
Contributor Author

Hi @aurangzaibsiddiqui
Apology for the delay response. I think this is a framework side issue,but this issue may come out from your code.
I don't know how you write this chat page. But maybe this is caused by the abuse of MediaQuery.of(Context). If the context is your page's context, your whole page will be rebuilt again in every frame of keyboard animation..which is very bad.

However, if you wrap something you need to change with a Builder widget, only the widget in builder will be rebuild during animation.

Here is the simple code using for framework side.

class TextFieldPage extends StatefulWidget {
  const TextFieldPage({Key? key}) : super(key: key);
  @override
  State<TextFieldPage> createState() => _TextFieldPageState();
}

class _TextFieldPageState extends State<TextFieldPage> {

  @override
  Widget build(BuildContext context) {
    print("page build");
    return Scaffold(
      body: Column(
        mainAxisAlignment: MainAxisAlignment.end,
        children: [
          const Spacer(),
          const Text('Keyboard animation', style: TextStyle(fontSize: 26)),
          const Spacer(),
          Builder(builder: (BuildContext builderCtx) {
            print('builder build');
            return SizedBox(
              /// If you use context, the whole page will be rebuild, you will see log "page build" and "builder build",
              /// but use builderCtx (after change you need to hot restart or recreate this page)
              /// you will only see log "builder build"
                height: 60 + MediaQuery.of(builderCtx).padding.bottom,
                child: const CupertinoTextField(
                  decoration: BoxDecoration(color: Colors.red),
                ));
          }),
        ],
      ),
    );
  }
}

@flutter-dashboard
Copy link

This pull request was opened against a branch other than main. Since Flutter pull requests should not normally be opened against branches other than main, I have changed the base to main. If this was intended, you may modify the base back to master. See the Release Process for information about how other branches get updated.

Reviewers: Use caution before merging pull requests to branches other than main, unless this is an intentional hotfix/cherrypick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.