849226 - Polish should have its own keyboard layout.

Status: VERIFIED FIXED

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

Description

[Staś Małolepszy :stas]

It should either be a separate layout like Spanish or Portuguese, or at least part of the "Other Latin Layouts" group.

Comment 1

[Michael Vines [:m1] [:evilmachines]]

ni? on product team for this one.

Comment 2

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

Taking.

I would suggest we have a separate layout for Polish if it is the language of one of our target markets.

Thanks.

Comment 3

[Michael Vines [:m1] [:evilmachines]]

(shira+ as Polish is one of the target markets for v1.0.1)

Comment 4

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

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

Pointer to Github pull-request

Comment 5

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

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

Polish keyboard layout has been added in this patch.

1. Add "ą" (a Polish diacritic) 
   - The other diacritics are based on English layout.
   - Please help let me know if I miss anything which is required in Polish.

2. Please note that it seems we don't have dictionary for Polish. So, I did not enable word suggestion for Polish.

Dear David, Stas,

Could you please review this patch?
Thanks.

Comment 6

[Staś Małolepszy :stas]

Attachment: Differences between Rudy's branch and mine

Rudy, thanks for working on this!

I took your patch and made a few edits, taking into the account the feedback I got from the Polish community.  Attached is the diff between your branch and mine.  I changed the order of the alt chars and removed some that are never used.

My code lives at https://github.com/stasm/gaia/compare/849226-polish-keyboard-layout

The biggest change that I'd like to get your input on is the complete removal of the 10th key on the second row of the keyboard.  Polish doesn't have any special punctuation mark to put there, and it definitely shouldn't be '.

I thus removed the key completely and applied text-align: center; to the keyboard-row class.

Everything works as expected.

Comment 7

[Staś Małolepszy :stas]

Attachment: Screenshot of text-align: center;

Comment 9

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

Attachment: The borde of highlighted "l"

Hi Staś,

Generally, your patch looks good.
However, we did have the assumption that we have 10 keys on each key row and have some CSS tweaks based on that assumption.

e.g. if you click "l", (now the 9th key on 2nd row) you would find the highlighted effect is rendered without the right border. (see the attachment)

We could solve this by:

1. (recommended) can I add any other symbol which is often used in Polish language to the 2nd row. That would require no UI change and is safe to land.

2. We would need to modify the CSS for polish layout only, which would need UI code change, both JS and CSS, I think.

What do you think?
Thank you.

Comment 10

[Staś Małolepszy :stas]

(In reply to Rudy Lu [:rudyl] from comment #9)

> e.g. if you click "l", (now the 9th key on 2nd row) you would find the
> highlighted effect is rendered without the right border. (see the attachment)

I see, understood.

> 
> We could solve this by:
> 
> 1. (recommended) can I add any other symbol which is often used in Polish
> language to the 2nd row. That would require no UI change and is safe to land.

Let's go for this option, as it clearly involves much less risk.

I think we could go for a colon as the additional key.

Let me commit some changes to my branch and I'll re-ask for the review when I'm done.

Thanks, Rudy!

Comment 11

[Kev Needham [:kev]]

Per follow on comments to comment #1, the Polish locale should have its own keyboard layout. (I know, redundant, but clearing the needinfo)

Comment 12

[Staś Małolepszy :stas]

Attachment: Polish keyboard layout with colon as the extra punctuation key

Hey Rudy, can you review this?  If it looks good, I can rebase and make a pull request.  Or feel free to pull from my branch and merge your PR.

Thanks!

Comment 13

[Bartosz Piec]

Will this patch make Polish layout the default after setting the language of Gaia to Polish?

Comment 14

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

Comment on attachment 724050 [details]
Polish keyboard layout with colon as the extra punctuation key

r=me.

Thanks for your work.
Please help squash your commits into one single commit.

Comment 15

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

(In reply to Bartosz Piec from comment #13)
> Will this patch make Polish layout the default after setting the language of
> Gaia to Polish?

Hi Bartosz,

Thanks for the reminding.
Yes, in FTU (First Time User Experience) we have the mechanism to change the default keyboard layout after the user language is changed.

However, in general settings, we did not implement that yet. 
That should be handled in Bug 827592.

Stas,

Hi I will change shared/resources/keyboard_layouts.json for FTU to change the default keyboard layout correctly.
Will send a patch soon.

Thank you.

Comment 16

[Staś Małolepszy :stas]

Attachment: Complete patch (with FTU support)

Rudy:  here's the pull request I opened with all changes I'd like to make.

An additional change since my last patch is the addition of Polish to shared/resources/keyboard_layout.json.  I also edited build/settings.py to use GAIA_DEFAULT_LOCALE to set the keyboard layout accordingly.

Let me know if these changes look good, and thanks for your help!

Comment 17

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

Comment on attachment 724346 [details] [review]
Complete patch (with FTU support)

r=me. Looks great. :)

Stas,

Thanks.

Comment 18

[Staś Małolepszy :stas]

Thanks, Rudy!  Fixed on master:  https://github.com/mozilla-b2g/gaia/commit/bc81fa5f3fffc9f6c8b216d19bf717fa77f2d628

I'll follow up with the branch landings shortly.

Comment 19

[Staś Małolepszy :stas]

v-train: https://github.com/mozilla-b2g/gaia/commit/62300c323daf64acec096d8822180ac435f8102b
v1.0.1: https://github.com/mozilla-b2g/gaia/commit/310e9f187de0eae1d5889cf2d2038dfd9fd8c11f

Comment 20

[Staś Małolepszy :stas]

I had to land a follow-up to fix the tbpl builds:

https://github.com/mozilla-b2g/gaia/commit/e72327872c67ead87a051992a2706856be065673

The issue is that my code was using a Python 2.7 syntax, while apparently the tbpl boxes use Python 2.6 (bug 850627).

I'll wait to see the new build pass, and I'll land the follow-ups on v1-train and v1.0.1.

Comment 21

[Staś Małolepszy :stas]

Follow up landings to fix build bustage on branches:

v1-train: https://github.com/mozilla-b2g/gaia/commit/acd6546cdd03284b3b8c12432fb9955868fc4c2b
v1.0.1: https://github.com/mozilla-b2g/gaia/commit/b34e726147f8e671ad8c538b50900ccfbffcb084

Comment 22

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

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/dc3add076016
Gaia   1e1c8c0ff2bc7f252fbe95016f108e38ece691a9
BuildID 20130314070204
Version 18.0
Unagi

I see the polish keyboard in the keyboard section and verified based on :
    alt: {
      a: 'ąáàâäåãāæ',
      c: 'ćčç',
      e: 'ę€éèêëē',
      i: 'ïíìîīį',
      o: 'óöòôōœø',
      u: 'üúùûū',
      s: 'śš$ß',
      S: 'ŚŠŞ',
      n: 'ńñ',
      l: 'ł£',
      y: '¥',
      z: 'żźž',
      '.': '?!…,',
      ',': '„”',
      ':': ';-—'

Verified in v1 train.

Comment 23

[Axel Hecht [:Pike]]

Taking the new string here for 1.0.1 and v1-train.