956172 - [keyboard refactor] backport 1.3 word suggestion changes to refactored keyboard

Status: RESOLVED FIXED

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

Description

[David Flanagan [:djf]]

We landed changes (a dismiss button) to the word suggestions bar of the keyboard in 1.3. These changes need to be backported to the refactored keyboard in progress in test_apps/demo-keyboard

Comment 1

[David Flanagan [:djf]]

Attachment: link to patch on github

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8359647 [details] [review]
link to patch on github

When you type, the suggestions flash. Looks likes the innerHTML gets cleared and only being set later. This is not the case on master with default keyboard.

f.e. type

Zoophy

Then press 't', the suggestions flash. Press backspace, t, backspace, t, etc. to repeat.

Comment 3

[David Flanagan [:djf]]

I didn't actually try this out on the device, so maybe I wasn't seeing it.

A flash might be necessary, however.  We want to erase the suggestions as soon as the user types a character because they are no longer relevant, and we can't display new suggestions until we've figured out what the suggestions are.  So it could be that there is just a bug in the master keyboard and it is not clearing the suggestions quickly enough.  I'll investigate.

Comment 4

[Jan Jongboom [:janjongboom] (inactive)]

Well, this doesn't look right at all :p Just try it on the device and you'll see it.

Comment 5

[David Flanagan [:djf]]

Comment on attachment 8359647 [details] [review]
link to patch on github

Trying it on the device I see what you mean.

I still maintain that the behavior of the new keyboard is more technically correct than the old keyboard because it removes suggestions as soon as they are no longer valid.

But I agree that the extra flashing is bad, so I've reverted to the old non-flashing behavior even though it means that old suggestions are displayed longer than they should be.  I have modified the code to prevent autocorrection if the displayed autocorrection is no longer valid.

Please review again.  All the changes in this update are in autocorrect.js

Comment 6

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8359647 [details] [review]
link to patch on github

r=me. Works fine now.

Comment 7

[David Flanagan [:djf]]

Merged to master: https://github.com/mozilla-b2g/gaia/commit/e93ba1f2df1ff98f748bd2d0f21ee7d61072fa19