912033 - [Keyboard] [New IME] Pop down animation is broken

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Description

[Jan Jongboom [:janjongboom] (inactive)]

Based on the CSS there should be a pop down animation just like the pop up anim for keyboard, but it does not work at the moment.

Comment 1

[Ivan Tsay (:ITsay)]

Need to fix

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch

It's dependent on bug 912028 because of the more sane flow introduced there, plus the tests.

What it does:

1. Don't hide the iframe where the 3rd party keyboard lives until pop down anim is played
2. Change animation to ease-in for pop down because you want slow->fast instead of other way around when disappearing

I tested this on GP Keon with mc and it looks nice and smooth.

Comment 3

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 811093 [details] [review]
Patch

Basically, this works and the animation looks good when hiding the keyboard.
Jan, thanks for the great work (again).

However, I noticed that there is a case that when you press [Home] button to make an app enter background, the keyboard would be hidden as well.
In this case, we did not handle the keyboard hiding good enough so the homescreen would be seen half-sized before the app is hidden. 
I am not sure when this is broken.

Do you think we can address that in this bug or we could open another bug to fix that?

Comment 4

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 811093 [details] [review]
Patch

I added an immediate hiding function to the PR. It's called hideKeyboardImmediately, please re-r? :-)

Comment 5

[Ivan Tsay (:ITsay)]

Hi Jan,

Would you please assign one of the target milestones (1.2 C3 ~ 1.2 C6) for this bug depending on time you need for resolving this one? We need the flag to check when this bug will be resolved. Thank you!

Ivan

Comment 6

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 811093 [details] [review]
Patch

Why is adding "notransition" necessary here?
I don't remember we have that in v1-train for immediately hiding the keyboard.

Could you help me understand it in more detail?
Thanks.

Comment 7

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 811093 [details] [review]
Patch

Because the pop down transition has always been broken, also on v1-train. Ergo: the problem of the transition playing when switching apps is a new one that appears due to fixing this bug.

Comment 8

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 811093 [details] [review]
Patch

It seems with this patch, the keyboard won't hide after it lose the focus?

Jan,

Could you please help check what went wrong?
Thank you.

STR
===
1. Open contact app
2. Click "+" to add a contact.
3. Click on the input field.
4. Click on the header or the [x] on the upper left => The keyboard won't be dismissed.

Comment 9

[Jan Jongboom [:janjongboom] (inactive)]

Hm, it's any issue that shows sometimes. It's because we get mozbrowserresize very often, and sometimes even after we hide the keyboard due to blur. However: when that event comes in:

    if (this.keyboardFrameContainer.classList.contains('hide') ||
        this.keyboardFrameContainer.dataset.transitionIn === 'true') {
      this.showKeyboard(updateHeight);

we cancel the blur and show it. So i think we should never open the keyboard if mozbrowserresize tells us to, but always rely on focus/blur events. I'll see if it has any nasty side effects.

Comment 10

[Kan-Ru Chen [:kanru] (UTC+9)]

You get mozbrowserresize because the keyboard app requested it. Maybe it's a issue that the keyboard requests too much. However I think this event should not prevent the keyboard manager to show/hide the keyboard so I agree that we always rely on focus/blur events.

Comment 11

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 811093 [details] [review]
Patch

Alright, all should be well now. There is an issue where (sometimes) the keyboard used to flicker, but because of changed behavior it now starts to slide down and then slides up immediately after switching elements. That's addressed in bug 931005.

Changes in window_manager were required so we hide the keyboard before closing the app, and the app will resize first so we don't hide a half app (cause keyboard already closed and the app isn't thus leaving half the screen open).

Comment 12

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 811093 [details] [review]
Patch

Ask for Gary's help to evaluate this patch.

Comment 13

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Jan,
   I've comment on github, looking forward to discuss with you on github.

   nice work!

Comment 14

[Jan Jongboom [:janjongboom] (inactive)]

Responded on GH (and rebased against master as well).

Comment 15

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Comment on attachment 811093 [details] [review]
Patch

r=me,
thanks for your great work!

Comment 16

[Jan Jongboom [:janjongboom] (inactive)]

Landed in https://github.com/mozilla-b2g/gaia/commit/a1c1ac4c03367291b29d2b5bc62a80196cfbb71a

Comment 17

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 a1c1ac4c03367291b29d2b5bc62a80196cfbb71a
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 19

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Jan,
   I've fixed the conflict on this pull request and I am waiting for travis. (https://github.com/mozilla-b2g/gaia/pull/13465)
   Do you want to do it by yourself or just use my pr?

Comment 20

[Jan Jongboom [:janjongboom] (inactive)]

Merge it!

Comment 21

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Travis passed.
https://travis-ci.org/mozilla-b2g/gaia/builds/13627560

Landed in V1.2
https://github.com/mozilla-b2g/gaia/commit/d4023a23f0305e1b0e26601dbe2ad9e0a18b39a6