934209 - Alternate keys list overflows

Status: RESOLVED FIXED

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

Description

[Trung Ngo]

Attachment: Screenshot from 2013-11-03 15:02:54.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130917102914

Steps to reproduce:

Try out the attached patch on comment #5 at bug #934198. Compile gaia with:

`make DEVICE_DEBUG=1 GAIA_KEYBOARD_LAYOUTS=en,vi`

Enable Vietnamese keyboard in Settings. Open any text field, switch to Vietnamese keyboard, press and hold the "u" key.

https://bugzilla.mozilla.org/show_bug.cgi?id=934198#c5


Actual results:

A list of candidates for "u" shows up but it's too long and overflows.


Expected results:

The list should adapt. Either by scrolling when the touch point reaches the left/right edges or by opening a two-dimensional bubble like in Android.

Comment 1

[arky [:arky]]

Do this happen only on Vietnamese keyboard or other existing keyboards as well?

Comment 2

[Trung Ngo]

I looks like we are the only language with enough combinations to trigger the bug. But who knows, what if someone decided to create a button for emoticons, listing lots of emoticons as alternative key?

Comment 3

[arky [:arky]]

Hey @jcarpenter What's your take on this ?

Comment 4

[arky [:arky]]

Perhaps if we reduce the size of tonal characters (Hint: ratio:x.x) then they might fit on one line. Possible work around?

Comment 5

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

This is a UX issue, so let's ask our UX designer, Carrie, to help on this topic.

Carrie,

Do you think we can have multiple-line alternative char menu?
Or what is the suggested way to handle this case?
Thanks.

Comment 6

[Carrie Wang [New account: [e-mail]]]

Hi guys,

We are currently working on keyboard design and coming out some suggestions on this issue.
We'll soon release the spec after internal review. Stay tuned!
Thanks.

Comment 7

[Trung Ngo]

Hey, I just want to note that this issue can still be found in the latest gaia.

Comment 8

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

Omega, could you help take this bug for design work as this would block bug 985334?
Thanks.

Comment 9

[Omega Feng [:Omega] [:馮於懋]]

Hi, we have 2 row design for this. Please see bug 983043 for the UX spec update. (p.14)

Comment 10

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

Hi Omega,

Yes, however per our previous offline meeting, we still need detailed design, especially the visual part, right?

Could you please give an estimate on when it will be ready?
Thanks.

Comment 11

[Omega Feng [:Omega] [:馮於懋]]

(In reply to Rudy Lu [:rudyl] from comment #10)
> Yes, however per our previous offline meeting, we still need detailed
> design, especially the visual part, right?

Right, Carol is doing this. ni? Carol for more details.

Comment 12

[Carol Huang [:Carol]]

Attachment: Alternate keys list_2 lines Visual Spec.zip

Hey Rudy,
The attachment is the Alternate keys menu visual spec for two lines.
Let me know if you have any question! thanks!!!

Comment 13

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

Carol,

Thanks for the updates.
The visual spec looks pretty good to me.

Comment 14

[arky [:arky]]

Thanks Carol, That looks very slick.

Comment 15

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Omega,

the spec hasn't pointed out how to handle below cases,
1. how about 8 keys?

1 2 3 4 5
6 7 8 

or

1 2 3 4
5 6 7 8 

2. How about 16 keys?

1 2 3 4 5 6 7 8
9 10 11 12 13 14 15 16 

or 

1 2 3 4 5 6
7 8 9 10 11 12
13 14 15 16

Please identify when to split to multiple lines and maximum num of lines.
Thanks.

Comment 16

[Omega Feng [:Omega] [:馮於懋]]

Hi George,
Please see bug 983043 for UX spec update.

Comment 17

[Omega Feng [:Omega] [:馮於懋]]

(In reply to Omega Feng [:Omega] from comment #16)
> Hi George,
> Please see bug 983043 for UX spec update.

It's in P.14.

Comment 18

[Carol Huang [:Carol]]

Attachment: Alternate keys list_2 lines Visual Spec_0528.zip

Hi Rudy,
There's some little changes on the Alternate Keys menu.
1. Make the menu corner to 1px.(both 1 line & 2 lines situation)
2. Change the size on "single overflow menu width" to 31px.

I've updated the Visual spec. Please let me know if you have any question! thanks!

Comment 19

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

I am fine with this visual update.
Carol, thanks for the work.

George, please be informed of this visual update if you're still working on this.
Thanks.

Comment 20

[Carol Huang [:Carol]]

Attachment: Alternate keys list_2 lines Visual Spec_0606.zip

Hi Rudy.

Update Alternate keys list menu Radius size 
2 lines menu:2px
1 key & 1 line:  will keep it rounded

thank you!

Comment 21

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

Got it.
Thanks for the visual update.

Comment 22

[Raniere Silva]

Attachment: Add alternate keys list

Rudy, do you know someone that can help me with the CSS?

Comment 23

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

Sorry but I am not sure who is going to help on this, maybe George?

Comment 24

[Raniere Silva]

Attachment: patch-screenshot.png

This is a screenshot of https://github.com/r-gaia-cs/gaia/commit/1be73ad92a12ec5ce53c47f371bfa30bd5fb320e.

Omega, about "Condition when two row candidates come in odd", the empty space should be at the right even when the key is at the right side of the keyboard?

Comment 25

[Raniere Silva]

Rudy, could you take a look at my patch in progress?

Comment 26

[Omega Feng [:Omega] [:馮於懋]]

(In reply to Raniere Silva from comment #24)
> Omega, about "Condition when two row candidates come in odd", the empty
> space should be at the right even when the key is at the right side of the
> keyboard?

Raniere, about "Condition when two row candidates come in odd", the empty space should be at the left when the key is at the right side of the keyboard.

Comment 27

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

I think it's too early to give any feedback on this patch, but please be informed,
 1. This is not only a visual refresh patch, it also involves interaction part, since now the alternatives menu contains 2 rows.
 2. We should avoid put inline CSS styles in JS code, for the complexity of this requirement, maybe we would define a set of rules for each combinations.

Comment 29

[Raniere Silva]

Rudy,

I make some changes at my patch. I still thinking the best way to replace CSS rule for first-child and last-child.

> 1. This is not only a visual refresh patch, it also involves interaction part, since now the alternatives menu contains 2 rows.

Could you be more clear about the changes involving interaction?

Comment 30

[Raniere Silva]

Attachment: screenshot of alternate keys after apply patch

Omega, do you know some one that can help me with the CSS?

Comment 31

[Omega Feng [:Omega] [:馮於懋]]

> Omega, do you know some one that can help me with the CSS?

The only guy who can help as I know is Rudy. :D

Comment 32

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

(In reply to Raniere Silva from comment #29)
> Rudy,
> 
> I make some changes at my patch. I still thinking the best way to replace
> CSS rule for first-child and last-child.
> 
> > 1. This is not only a visual refresh patch, it also involves interaction part, since now the alternatives menu contains 2 rows.
> 
> Could you be more clear about the changes involving interaction?

I mean the first section of page 13 of the spec,
https://bug983043.bugzilla.mozilla.org/attachment.cgi?id=8454327#page=13

But as our offline discussion in the email, I'll look into this issue, so don't worry about it.
:)

Comment 33

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

Attachment: WIP

WIP for this requirement.
There are some items to be addressed, I think it is ready to get some feedback from code and UI side.

Please help take a look.

ToDo:
UI part:
 1. Might need some tweaks for landscape mode.
 2. Need to check with VD about the top positioning of the menu, right now I just keep "1-row" menu the same as before, "2-row" would the same as the spec.
 3. The horizontal position might need some tweaks as well, especially for the first/last key.

Code level:
 1. Unit test are not addressed yet, and this might break some integration test, I suppose.

Comment 34

[Omega Feng [:Omega] [:馮於懋]]

Comment on attachment 8477271 [details] [review]
WIP

Two comments:

1) There is no buffer for touch area in this patch
   It's better to have some buffer like before:

 #6  #7  #8  #9
[#1] #2  #3  #4  #5
╭──╮
│  │

 #6  #7  #8  #9
 #1  #2  #3  #4 [#5]
                ╭──╮
                │  │

 #6  #7  #8  #9
 #1  #2  #3  #4 [#5]
                    ╭──╮
                    │  │

2) Similar to 1), the second row should have buffer as well.
   
 #6  #7  #8  #9
[#1] #2  #3  #4  #5
╭──╮
│  │

 #6  #7  #8  #9
 #1  #2  #3  #4 [#5]
                ╭──╮
                │  │

 #6  #7  #8 [#9]
 #1  #2  #3  #4 ╭──╮
                │  │

Comment 35

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

Comment on attachment 8477271 [details] [review]
WIP

The patch has been updated: 
 1. To address the comments around the interaction details.
 2. The positioning in landscape mode.

Omega, mind to look at it again?
Thanks.

Comment 36

[Omega Feng [:Omega] [:馮於懋]]

Comment on attachment 8477271 [details] [review]
WIP

Good job! Thanks! :)

Comment 37

[Carol Huang [:Carol]]

Attachment: alternative_flow_spec_0827.jpg

Hi Rudy,
For the position alternate key menu, please leave 0.4rem space if it's on the sides (see attachment). Thanks!

Comment 38

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8477271 [details] [review]
WIP

This looks good and thank you for spending time to give some thought on the architectures. So as we have discussed offline, I still some concerns between the separations between the new module and the current AlternativesCharMenuManager. We can definitely discuss more in the next iteration.

Comment 39

[Carol Huang [:Carol]]

Comment on attachment 8477271 [details] [review]
WIP

looks good!
But there's few minor improvement we need to do.

Let's file bugs for that later on.
Thank you, Rudy!

Comment 40

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

Comment on attachment 8477271 [details] [review]
WIP

Patched updated to address both code and UI review comments,
   Code
     - Update the naming of menu view.
     - Refine the menu related instances that manager holds.
     - Fix integration test and unit tests.

    UI:
    To address UI review comment for edge case.
    Other tweaks according to offline discussion.

Tim, could you help review this?
Thank you.

Comment 41

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8477271 [details] [review]
WIP

r=me after offline discussion.

So eventually I think we should still absorb all coordinate calculations (i.e. isMenuTarget() and getMenuTarget()) to view classes, but I agree we not to do it in this patch because since that means we need more code in render.js and it's harder to harden the code there.

Comment 42

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

Landed to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/7592d02b61490fe09f5cf4279b9441f7fb2a139a

--
Thank you all for the review.

Comment 43

[howie [:howie]]

Remove the feature b2g tag to match the new v2.2 scope.