Closed Bug 1003649 Opened 8 years ago Closed 7 years ago

[Keyboard UX update] Change IME selector to use value selector style instead of action menu style

Categories

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

x86
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(feature-b2g:2.0, b2g-v2.0 fixed)

VERIFIED FIXED
2.0 S2 (23may)
feature-b2g 2.0
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: rudyl, Assigned: rudyl)

References

Details

(Whiteboard: [p=1])

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #985851 +++

Right now, the IME selector use action menu style, which does not support 2 buttons at the bottom, so we might need to change that to value selector for 2 reasons,
 1. Conform to the spec, after discussed with Omega.
 2. To fulfill the requirement of bug 985851.
To double confirm, I'd like to consult building block expert on how we can customize action menu.

Arnau,

Do you know if it is possible to show 2 buttons (within the same row) at the bottom of action menu?
This is what we need for the keyboard layout menu: p8 of attachment8408812 [details].
Thanks.
Flags: needinfo?(arnau)
Unfortunately Action menu cannot use 2 buttons the way we have structured the HTML.
Neither is Value selector:
http://buildingfirefoxos.com/building-blocks/value-selector.html

Value selector needs to steps to trigger an option:
1. Select option.
2. Tap "Select button".

So in case you want to implement that screen using a Value selector you would need 3 buttons: Select, cancel and settings.

As Action menu could be difficult to teak, I would suggest two options:
1- Use action menu with a setting icon to the right of the header.
2- Change the behavior of value selector, so the action is triggered on tap (without having to go to "select" afterwards)
Flags: needinfo?(arnau) → needinfo?(ofeng)
(In reply to Arnau March  [:arnau] from comment #2)
> As Action menu could be difficult to teak, I would suggest two options:
> 1- Use action menu with a setting icon to the right of the header.
> 2- Change the behavior of value selector, so the action is triggered on tap
> (without having to go to "select" afterwards)

I prefer using Value Selector, since IME selection is for selecting a value.
And yeah, it will be great if Value Selector can change something:
1) If it's a single-selection <select> tag, tap an item to select and leave, and the bottom button is Cancel.
2) The bottom button can be customized, like multiple buttons and customizable button behaviors.

If the above changes cannot be done now, we use Action Menu with settings icon at top-right header.
Flags: needinfo?(ofeng)
Arnau, Omega,

Thanks for your suggestion.
I'll try to go with option 1 in Comment 3, that is using a customized "value selector" style menu for our IME selection menu, which would have 2 buttons, [Cancel] and [Settings].

Note that this won't change the system value selector behavior.
Status: NEW → ASSIGNED
I think this is ready to review.
In this patch, I created a new class ImeMenu here since we need a value-selector style for our IME selection menu instead of using the original ActionMenu.

Tim, could you help review this?

--
Omega, please help on the UX review.
Attachment #8420565 - Flags: ui-review?(ofeng)
Attachment #8420565 - Flags: review?(timdream)
Comment on attachment 8420565 [details] [review]
Patch V1 - pull request 19129

Hi Rudy,
Looks nice! However I found something worth to take look at:
1) Title of value selector: "Layout selection"
   It's better to use "Select"
2) Our Built-in IME strings are too long
   They cause 2 rows in value selector Maybe we can file another bug to discuss this.
3) When tapping "Settings" in value selector first time, it goes to "Settings" and then goes to "Keyboards"
   It should go to "Keyboards" directly.
4) In "Settings > Keyboards", the top-left icon is "X" (it's correct). Tap "Built-in Keyboard" -> Tap "<" to go back -> the "X" in "Settings > Keyboards" change to "<" -> KO
   It should keep "X".
Attachment #8420565 - Flags: ui-review?(ofeng) → ui-review-
Hi Omega,

Thanks for your feedback.

(In reply to Omega Feng [:Omega] from comment #6)
> Comment on attachment 8420565 [details] [review]
> Patch V1 - pull request 19129
> 
> Hi Rudy,
> Looks nice! However I found something worth to take look at:
> 1) Title of value selector: "Layout selection"
>    It's better to use "Select"

This should be easy to modify.

> 2) Our Built-in IME strings are too long
>    They cause 2 rows in value selector Maybe we can file another bug to
> discuss this.

Yeah, agree. The long name of "Built-in keyboard" did cause some problems before, and we should come out with a better name. Hope to handle this with an separate bug.

> 3) When tapping "Settings" in value selector first time, it goes to
> "Settings" and then goes to "Keyboards"
>    It should go to "Keyboards" directly.

I guess this is by the current design of settings, and we will tackle this issue after settings app refactoring.

> 4) In "Settings > Keyboards", the top-left icon is "X" (it's correct). Tap
> "Built-in Keyboard" -> Tap "<" to go back -> the "X" in "Settings >
> Keyboards" change to "<" -> KO
>    It should keep "X".

And this one should be related to bug 1005827.
Will track this bug as a dependency of the user story bug.
Hi Arthur,

For comment 6 point 3,
  3) When tapping "Settings" in value selector first time, it goes to "Settings" and then goes to "Keyboards"

Do we have a bug to depend on?

Thanks.
Flags: needinfo?(arthur.chen)
Yes, we have https://bugzilla.mozilla.org/show_bug.cgi?id=1007600 tracking this issue.
Flags: needinfo?(arthur.chen)
Arthur, thanks.

The above 2 issues have been set at dependencies as follows,
https://bugzilla.mozilla.org/show_bug.cgi?id=985851#c4
Comment on attachment 8420565 [details] [review]
Patch V1 - pull request 19129

Please fix feedback from Omega before landing.
Attachment #8420565 - Flags: review?(timdream) → review+
Omega, do you think we could land this without the settings button, with 1) addressed?
If yes, I would send an updated pull request for this.

Thank you.
Flags: needinfo?(ofeng)
(In reply to Rudy Lu [:rudyl] from comment #12)
> Omega, do you think we could land this without the settings button, with 1)
> addressed?
> If yes, I would send an updated pull request for this.

Yeah, we can land it for now, and add the Settings button back when it's ready.
Flags: needinfo?(ofeng)
This is the version that does not contain the settings button.

Omega,

Could you please confirm this is what we want for now?
Thanks.
Attachment #8423710 - Flags: ui-review?(ofeng)
Comment on attachment 8423710 [details] [review]
Patch V1 - without the settings button

ui-review+, but it seems to crash easily on my Helix. Don't know why... You can check my phone if you'd like to.
Attachment #8423710 - Flags: ui-review?(ofeng) → ui-review+
Comment on attachment 8423710 [details] [review]
Patch V1 - without the settings button

Carry forward the r+.
Landed to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/3afc852b8579d22072919408f6c2e39b167590c8

--
Could not repro the phone crash issue at my side, so seems irrelevant to this change.
Attachment #8423710 - Flags: review+
Assignee: nobody → rlu
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
I have verified this patch.
But I think we need to adjust the value selector UI.
Attach the screenshot. (2014-05-21-15-57-11.png)

* Build information:
 - Gaia      c462d9183d294a2d8ecc472f593ea8cfa15bc9de
 - Gecko     https://hg.mozilla.org/mozilla-central/rev/9d8d16695f6a
 - BuildID   20140520160203
 - Version   32.0a1
Status: RESOLVED → VERIFIED
Blocks: 1013837
No longer blocks: 1013837
Remaining work -> Bug 1013837
Whiteboard: [ucid:SystemPlatform54, 2.0, ft:system-platform], [p=1] → [p=1]
Target Milestone: --- → 2.0 S2 (23may)
Depends on: 1013837
feature-b2g: --- → 2.0
You need to log in before you can comment on or make changes to this bug.