796600 - If the number of language support for keyboard is too long, it will cause a graphic defect on the list selection for the language of the keyboard.

Status: VERIFIED FIXED

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

Description

[GH to BZ]

[GitHub issue by nhirata on 2012-09-05T18:09:29Z, https://github.com/mozilla-b2g/gaia/issues/4364]
## Environment :
Otoro phone, build 2012-09-05
Taken from default.xml in b2g-distro: 
* "platform_build" revision= 76e88b579737d7b5078063dcbec60c2ad2c70465
* "gaia" revision= 279a13474955c1bc58082c8f3c6f54cf09d8c396 
* "mozilla-central" revision= 566ea2b9de39e8b9b18513931fc1cb1825535f3a
* "gonk-misc" revision= a9b6133e492017d34703ae3f24ea1fa9349cbcce
  

## Repro :
1.  go to settings -> keyboard
2. checkmark all languages
3. hit home
4. launch browser
5. tap into url bar
6. long tap on the world icon

## Expected :
1. the drop down list for keyboards does not have a graphics defect.

## Actual :
1. the drop down list for keyboards does have a graphics defect.
2. screen shot http://cl.ly/image/313Q06381L26

## Note :

Comment 1

[Armen [:armenzg]]

> 6. long tap on the world icon

The world icon does not show up. It seems that it got removed.

I'm trying to find an app that could allow me to try to change the keyboard but no luck so far.

Comment 2

[Armen [:armenzg]]

Should this be marked INVALID since we don't have multiple keyboard layouts working? (bug 806387).

Comment 3

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

No it should not.  We need this feature, it's not suppose to be removed.

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

note: you can't select some of the languages when this happens

Comment 9

[Walter Chen[:ypwalter][:wachen]]

renominating since there are few reporting of this bug recently.

Comment 10

[Chris Kreinbring [:CKreinbring]]

Still repros on Unagi version 20130130070201.
Kernel: Dec 5
Gecko:  http://hg.mozilla.org/releases/mozilla-b2g18/rev/4593f3e765eb
Gaia:   f7f5a0cd17e3d04308cc5850b254947e127122b9

Comment 11

[Chris Kreinbring [:CKreinbring]]

Repros in Unagi version 20130225070200.
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/3a5a27992a75
Gaia: 5691a16fff8e1403c75ed9d6f3a443b7e58198c6
Kernel Date: Dec 5
Test Suite: Keyboard & Text-Handling
UCID: kbandtext-007
https://moztrap.mozilla.org/runtests/run/859/env/305/?pagenumber=1&pagesize=50&sortfield=order&sortdirection=asc&filter-id=3930&filter-suite=172

Comment 12

[Leo]

If we cannot restrict the number of keyboard layouts, then  putting scroll option in Language menu list pop up on pressing globe(language) icon can be a possible solution.

To remove the graphics defect,one possible solution is like this. 

1.On long pressing globe(language) icon, the language list menu  is displayed.
2.On releasing globe icon, the language list menu still displays and doesn’t   disappear.
3.The list menu after that can be scrolled to display all the languages.
4 When any language is selected from List, the menu list then disappear.

Please give your opinion for the above solution or any suggestion.

Comment 13

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

Hi,

I think the proposal in Comment 12 looks good to me.
BTW, for when to make the menu list disappear, we may add a timeout, so that it would disappear after say, 3 seconds or so.

[Also cc UX people]

Comment 14

[Casey Yee]

Rather than using the pop-up list selection we should use our standard value selector pattern:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/UX/Building_blocks/Value_selector

This will accommodate the expanded list and avoid the overrun in the UI.

Comment 15

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

Hi Casey, Francis,

Another concern related to this one is, right now we have several keyboard layouts put in a single group of setting entry.
Please refer to the attachment.

Could you please comment on if we need to put each keyboard layout in a specific setting entry?

Comment 16

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

Attachment: screenshot for keyboard layout grouping

The attachment for Comment 15.

Comment 17

[Francis Djabri [:djabber]]

(In reply to Rudy Lu [:rudyl] from comment #15)
> Hi Casey, Francis,
> 
> Another concern related to this one is, right now we have several keyboard
> layouts put in a single group of setting entry.
> Please refer to the attachment.
> 
> Could you please comment on if we need to put each keyboard layout in a
> specific setting entry?

Agreed - there should be a separate settings entry for each separate keyboard layout.

Comment 18

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

Hi Francis,

Thanks for the your feedback.
The follow-up bug has been filed as Bug 867883 - Each keyboard layout should have a separate setting entry.

Comment 19

[Francis Djabri [:djabber]]

(In reply to Casey Yee [:cyee] from comment #14)
> Rather than using the pop-up list selection we should use our standard value
> selector pattern:
> https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/UX/
> Building_blocks/Value_selector
> 
> This will accommodate the expanded list and avoid the overrun in the UI.

Hi Rudy, 

I agree with this approach - just wanted to make sure that the Single Select value select style was being used to give the user the selection.

Comment 20

[Francis Djabri [:djabber]]

Attachment: Single Select Value Selector

Wireframe to illustrate the point above

Comment 22

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

Hi Francis,

Do you think we can stick to the original design, if we got some technical problem on implementing Comment 14.
Please refer to the "languages" section of this design image,
https://mozilla.box.com/system/1/867862570/8021730498/1

The difficulty is that we need to use <select> element to implement Comment 14, and it would need to be "focused" to show the full-screen value selector.
After the <select> is focused, it would grab the focus from the original text input that originally triggered the keyboard and caused the keyboard to disappear.

If we used the original design, we can avoid this problem.
The alternative is to move the whole value selector from Gaia system app. to Gaia keyboard app., which we would like to avoid to reduce code duplicate.

Thanks.

Comment 23

[Francis Djabri [:djabber]]

(In reply to Rudy Lu [:rudyl] from comment #22)
> Hi Francis,
> 
> Do you think we can stick to the original design, if we got some technical
> problem on implementing Comment 14.
> Please refer to the "languages" section of this design image,
> https://mozilla.box.com/system/1/867862570/8021730498/1
> 

Hi Rudy, I'm afraid that the link isn't working for me - it just takes me to:
https://www.box.com/files/0/f/864533388 

and I don't see a languages section there. 

I think I can follow what you're saying, however. I don't have a problem with the original design, provided you mean the design suggested in comment 12, and it would also reactive bug #879676 - in which case I'd recommend that selection list would need to change its width dynamically to match the width of the longest label, as Mihai recommends in option 1 of comment 3 of that bug. 

I've also cc'ed Neo on here as he's transitioning to taking ownership of the keyboard UX.

Comment 24

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

Attachment: design spec for keyboard layout selection

Hi Francis,

I just re-attached the design spec to confirm we are on the same page.
I am going to try to modify this as Comment 12 suggested and solve the list width issue at the same time.

Thanks for the feedback.

Comment 25

[Francis Djabri [:djabber]]

(In reply to Rudy Lu [:rudyl] from comment #24)
> Created attachment 762532 [details]
> design spec for keyboard layout selection
> 
> Hi Francis,
> 
> I just re-attached the design spec to confirm we are on the same page.
> I am going to try to modify this as Comment 12 suggested and solve the list
> width issue at the same time.
> 
> Thanks for the feedback.

Hi Rudy, that sounds great, looking forward to checking it out.

Comment 26

[Alex Keybl [:akeybl]]

Rudy - do you have an ETA for this landing? It'll need to land to master before 1.1, and all of that should take place before 6/24.

Comment 27

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

Hi, Alex,

Yes, should be able to send the patch today and to land in a day or two after review.
Thank you.

Comment 28

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

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10572

Pointer to Github pull-request

Comment 29

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

Comment on attachment 766637 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10572

Hi David,

Please help review this patch to make the keyboard layout menu scrollable when the layout list is quite long.
I just added a simple logic to scroll a div container (.menu-container) when the touch near the edge of the keyboard menu.

Thanks.

Comment 30

[David Flanagan [:djf]]

Comment on attachment 766637 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10572

I wish we had a better way to do this menu, but at this point we just need to fix it.

I'm giving r- here because I find that I cannot reliably use the new menu. I think there are some easy (I hope) fixesthat would improve it a lot:

1) Allow me to control the menu even if my finger moves out of it.  If you do this, then maybe the area that triggers scrolling can be just above the top item and just below the bottom item.

2) List the avaialable layouts from bottom to top, and begin with the menu scrolled all the way to the bottom.  Otherwise, when there is a long list of layouts and I want to scroll down, I have to press the button, then move my finger up onto the menu and then move it down again to scroll down. It seems very counter-intuitive.

3) Maybe use a timer to trigger scrolling?  Sometimes it seems to scroll very quickly. I'd think that 3 per second is fast enough.

Comment 31

[David Flanagan [:djf]]

Rudy,

Instead of working to get the scrolling working better, would it be easier to just allow multiple columns of keyboard layouts?  3 columns of 6 or 8 ought to be enough with no scrolling.  And as soon as we get rid of the "otherlatins" layout group most users will never have more than 3 or 4.

Comment 32

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

Comment on attachment 766637 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10572

Hi David, 

Thanks for the review.
I have refined the patch with mainly the following 3 points,
 1. Add LOCKED_AREA so the menu will not be easily dismissed
 2. Adjust the scrolling speed
 3. Will do continuous scrolling if you stick to the edge

--
Hope this will make the usability a lot better.

 - As you mentioned, this might be a temporary solution for v1.1 since we're going to redesign/implement the keyboard menu for 3rd-party keyboard in v1.2.
 - I did not take multi-column (Comment 31) since I think scrolling is much more intuitive and this has been approved by UX people.

Comment 33

[David Flanagan [:djf]]

Comment on attachment 766637 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10572

Rudy,

See my comments on Github. Please check what happens if the user taps Home or Sleep while scrolling the keyboard. I worry that this will cause an infinite series of setTimeout() calls and the scrolling code will never exit.

If that does not happen, or if you fix that issue, then r+

The usability of this patch is *much* better than the last one. There are still improvements that could be made, but this is such a corner case that it is not worth fixing it any more since we're going to re-do it for v1.2.

Comment 34

[David Flanagan [:djf]]

Comment on attachment 766637 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10572

I'm still feeling nervous about this, so I went ahead and added console.log() statements to the scroll up and scroll down cases, and confirmed that they keep running even if you tap the sleep button while scrolling the menu.  (I'm testing on a unagi without a hard home button, so I can't press home while still holding the menu, but I'm assuming it would be the same with a dedicated home button).

So converting to an r-.

Comment 35

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

Comment on attachment 766637 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10572

Hi David,

Thanks for the review (again) and sorry that it still had some parts to improve and fix.
 
I have refined it mainly for these points: 
  1. Stop the scrolling if scroll to top/end of the menu
  2. Stop the timeout for scrolling when the keyboard is hidden (by Home
     button or Power button)
  3. Also adjust the time/step for scrolling, to make it smoother

Please help check the changes in this commit. (will squash them after review)
https://github.com/RudyLu/gaia/commit/65a6ccfc45f1a4b94f0e03d97ddf18c49bcbfc40

Thank you!

Comment 36

[David Flanagan [:djf]]

Comment on attachment 766637 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10572

Looks good to me.  Rather than add a new event handler that calls reset even if the menu is not displayed, I'd suggest that you just check for document.mozHidden in your scroll() method.

Comment 37

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

Merged to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/94b7c5ac0cef9439eeee8ca1421c52ccc10421a8

--
This patch would be a lot easier to uplift if we have Bug 888137 landed first.
Might need some extra time to resolve the conflict if we don't.

Comment 38

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

Uplifted to Gaia v1-train,
0838ce73999c04acf5479abae0b836034c9e963b

Comment 39

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

v1.1.0hd: 0838ce73999c04acf5479abae0b836034c9e963b

Comment 42

[Walter Chen[:ypwalter][:wachen]]

Verified in Unagi PVT V1Train 20130815 build

Gaia:     0f1f1ab0ab31a1df8a780baa048b5e7b2854205d
  B-D     2013-08-15 06:32:03
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18/rev/692d3414bb12
BuildID   20130815041201
Version   18.0