867883 - Each keyboard layout should have a separate setting entry

Status: RESOLVED FIXED

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

Description

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

Right now, in Settings app > Keyboard, we have some keyboard layouts grouped as a single entry, like "Other Latin scripts".

This should be refined for better UX and easier handling Bugs, such as Bug 828237 and Bug 867175.

Note: this is for v1, for v2, we're going to have a different design, logged Bug 856910.

Comment 1

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

** For triage **
Nominate this issue as leo?, since German and Serbian (Cyrillic) are the main languages for our target markets, and right now they are put into some grouping.
German -> "Other latin scripts" group
Serbian (Cyrillic) => "Cyrillic scripts" group

All other languages in the same group will be activated when the user wants to enable German, which is not good user experience.

Comment 2

[Alex Keybl [:akeybl]]

Not blocking since it's still functional and this isn't a partner blocker, but we'd take a low risk patch depending on where we are in the cycle. We'll ask for Francis's help in figuring out what the UX should be like.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Alex Keybl [:akeybl] from comment #2)
> Not blocking since it's still functional and this isn't a partner blocker,
> but we'd take a low risk patch depending on where we are in the cycle. We'll
> ask for Francis's help in figuring out what the UX should be like.

I assume the UX would be one row per layout we want. What else do you expect?

In the meantime I suggest that people can work on this.

Comment 4

[Francis Djabri [:djabber]]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #3)

> I assume the UX would be one row per layout we want. What else do you expect?
> 
> In the meantime I suggest that people can work on this.

Agreed

Comment 5

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Removing the keyboard layout groups and matching each keyboard layout to its relevant language (1-to-1 mapping), was an idea I also discussed with David Flanagan for the patch of bug 828237: for example, the user might not be interested to have 6 enabled keyboards (i.e. otherlatins group) if he only wants the German one.

In order to have individual keyboard layouts showed one per row, the layout groups logic needs to be changed quite significantly, making groups redundant in the end.

However, the keyboard groups idea might be useful for cases like the one highlighted in bug 868001, when non-latin keyboard layouts require at least one latin layout active for authentication on different services.

My suggestion would be to keep the layout groups and have them map to only one layout (as implemented now for English, Spanish, etc) to further accommodate multiple layouts for a single language (cf. example above for bug 868001 case) and if no one is considering this bug, I would be happy to take it.

Rudy (or anyone else working on the Keyboard app), any thoughts on this?

Comment 6

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

Hi Mihai,

The current group idea is about map one single setting entry to one or multiple keyboard layouts.
i.e.
  "keyboard.layouts.otherlatins" -> {"French Keyboard", "German Keyboard", ...}

I think we should change this to one-to-one mapping.
For Bug 868001, we can define some dependency logic to include English layout (or others) when only non-latins are enabled. That logic will change the settings entry, say set both "keyboard.layouts.english" and "keyboard.layouts.arabic" as true in this case.

This will make keyboard app independent of the layout dependency problem.
--

BTW, feel free to take it.
Thanks for your input.

Comment 7

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

(In reply to Rudy Lu [:rudyl] from comment #6)
> Hi Mihai,
> 
> The current group idea is about map one single setting entry to one or
> multiple keyboard layouts.
> i.e.
>   "keyboard.layouts.otherlatins" -> {"French Keyboard", "German Keyboard",
> ...}
> 
> I think we should change this to one-to-one mapping.
> For Bug 868001, we can define some dependency logic to include English
> layout (or others) when only non-latins are enabled. That logic will change
> the settings entry, say set both "keyboard.layouts.english" and
> "keyboard.layouts.arabic" as true in this case.
> 
> This will make keyboard app independent of the layout dependency problem.
> --
> 
> BTW, feel free to take it.
> Thanks for your input.

Thanks for the suggestions, Rudy! I'm going to start working on this.

Comment 8

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

(In reply to Mihai Cirlanaru [:mcirlanaru] from comment #7)
> (In reply to Rudy Lu [:rudyl] from comment #6)
> > Hi Mihai,
> > 
> Thanks for the suggestions, Rudy! I'm going to start working on this.

Alternatively, we might want to concentrate the settings into one key in the mozSettings, like "keyboard.layout" which stores an array containing which each item represents a layout. I am fine if we do this in this bug, or in another bug when we land 3rd party keyboard support.

Comment 9

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #8)
> (In reply to Mihai Cirlanaru [:mcirlanaru] from comment #7)
> > (In reply to Rudy Lu [:rudyl] from comment #6)
> > > Hi Mihai,
> > > 
> > Thanks for the suggestions, Rudy! I'm going to start working on this.
> 
> Alternatively, we might want to concentrate the settings into one key in the
> mozSettings, like "keyboard.layout" which stores an array containing which
> each item represents a layout. I am fine if we do this in this bug, or in
> another bug when we land 3rd party keyboard support.

Hi Tim, thanks for the suggestion, I think having only a "keyboard.layout" setting as a map between layout names and keyboards (i.e. {'english': 'en', 'french': 'fr', ...}) is a very good approach, and I will add it for the patch of this bug.

Comment 10

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

(In reply to Mihai Cirlanaru [:mcirlanaru] from comment #9)
> > Alternatively, we might want to concentrate the settings into one key in the
> > mozSettings, like "keyboard.layout" which stores an array containing which
> > each item represents a layout. I am fine if we do this in this bug, or in
> > another bug when we land 3rd party keyboard support.
> 
> Hi Tim, thanks for the suggestion, I think having only a "keyboard.layout"
> setting as a map between layout names and keyboards (i.e. {'english': 'en',
> 'french': 'fr', ...}) is a very good approach, and I will add it for the
> patch of this bug.

Why would you store such info in mozSettings? My suggestion was about having all the enabling flags contained in one mozSettings entry, e.g. ['en-US-qwerty','fr-FR-awerty']. thus |var isEnabled = (settingsArr.indexOf(layoutName) !== -1);|.

Comment 11

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #10)
> (In reply to Mihai Cirlanaru [:mcirlanaru] from comment #9)
> > > Alternatively, we might want to concentrate the settings into one key in the
> > > mozSettings, like "keyboard.layout" which stores an array containing which
> > > each item represents a layout. I am fine if we do this in this bug, or in
> > > another bug when we land 3rd party keyboard support.
> > 
> > Hi Tim, thanks for the suggestion, I think having only a "keyboard.layout"
> > setting as a map between layout names and keyboards (i.e. {'english': 'en',
> > 'french': 'fr', ...}) is a very good approach, and I will add it for the
> > patch of this bug.
> 
> Why would you store such info in mozSettings? My suggestion was about having
> all the enabling flags contained in one mozSettings entry, e.g.
> ['en-US-qwerty','fr-FR-awerty']. thus |var isEnabled =
> (settingsArr.indexOf(layoutName) !== -1);|.

Ohhh, I see, then I misunderstood your suggestion. It makes more sense now :) Thanks Tim!

Comment 12

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #10589 - Split kb layout groups

Remove the 'otherlatins' and 'cyrillic' keyboard layout groups and create individual language-specific entries for keyboards.

Note: this keeps the settings logic as before (i.e. 'keyboard.layouts._') and does not create a settings array under 'keyboard.layouts' (as suggested in previous comments of this bug). I would suggest filing a follow-up bug to tackle this.

Comment 13

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

Comment on attachment 766976 [details]
Pull Request #10589 - Split kb layout groups

Generally, I think this is good that we should have this fixed first to get rid of the grouping like "otherlatins".

However, serbian is still used in keyboard app. as a grouping entry for both "Serbian (latin)" and "Serbian (Cyrillic).
Do you think we can remove that in this patch?

Thanks.
Please ask for review again so that I will get notified.

Comment 14

[Julien Wajsberg [:julienw]]

I think keeping these for Serbian was intended, as it makes sense. Maybe UX thinks otherwise ?

Comment 15

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

Please be informed that I am fine with enabling "Serbian (latin)" & "Serbian (Cyrillic)" at the same time in the settings app or during build time (l10n customization).

I just hope that this logic could live in settings and build time script and maybe FTU instead of keyboard app., since that should not be in the control of keyboard.

For v1.1, I will take Mihai's solution if he thinks the alternative needs a much more complicated patch.

Comment 16

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Hi Rudy, Julien,

The Serbian keyboard layout group is indeed intentional and the whole layout group idea makes sense for such cases where a certain language has multiple (popular) keyboard layouts (see bug 868417 for another example besides Serbian).

(In reply to Rudy Lu [:rudyl] from comment #15)
> I just hope that this logic could live in settings and build time script and
> maybe FTU instead of keyboard app., since that should not be in the control
> of keyboard.

It would make sense to have the one-to-one mapping (visually grouped eventually) in the Keyboard section of Settings and the layout groups idea for the FTU, build customizations, or when changing the language. However, I think that this would involve a more extensive change to the mozSettings 'keyboard.layouts._' logic (i.e. to store individual keyboards instead of groups) and it might be better to just treat it in a follow-up bug.

Comment 17

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

Comment on attachment 766976 [details]
Pull Request #10589 - Split kb layout groups

I think I could give this a r+ and we should ask for approval to land this low-risk patch to v1.1.

Mihai, Julien,

Thanks for your work and feedback.

Comment 18

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Thanks, Rudy!

Landed on master:
https://github.com/mozilla-b2g/gaia/commit/7381529227212a274e04959d5a185ffd191c7435

Comment 19

[David Flanagan [:djf]]

This is great that we finally got this fixed.  Setting leo? because of comment 17.

Comment 20

[Dietrich Ayala (:dietrich)]

The user impact does not seem enough to take on 1.1 so late. Let's ship this in 1.2.