958919 - [Keyboard] Latvian keyboard

Status: RESOLVED FIXED

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

Description

[Binyamin]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/31.0.1650.63 Safari/537.36

Steps to reproduce:

Pull request in https://github.com/mozilla-b2g/gaia/pull/14142

Comment 1

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: GH Pull Request

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

Hi Binyamin, next time if you open a pull request; please click 'Add an attachment', paste the link, and then set the reviewer flag to '?' and then to one of the suggested reviewers. I did it for you now :-)

Comment 3

[Jan Jongboom [:janjongboom] (inactive)]

Hi Raivis, can you check whether this keyboard is correct? If not, can you redirect it to someone else in the Latvian community?

Comment 4

[Binyamin]

(In reply to Jan Jongboom [:janjongboom] from comment #3)
> Hi Raivis, can you check whether this keyboard is correct? If not, can you
> redirect it to someone else in the Latvian community?

My mother language is Latvian. And my reported Latvian keyboard pull request is correct and mostly common - in http://en.wikipedia.org/wiki/Latvian_language#Latvian_on_computers you can see an optional rarely used Latvian keyboard layout. The rarely used keyboard is recently used by http://translate.google.com/#lv/en/, but definitely must be reported as issue, since none Latvian speaker actually uses such a keybord.

https://github.com/laukstein/gaia/commit/9db743b1b7a3211913252e2d872f7ac8c074c1b8 has the correct and most common Latvian keyboard layout.

Comment 5

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

Comment on attachment 8359099 [details] [review]
GH Pull Request

Looks like the code needs to be updated to bring the latest changes.
In 'shared/resources/keyboard_layouts.json', we have changed the format from "appOrigin" to "appManifestURL".

Comment 6

[Binyamin]

(In reply to Rudy Lu [:rudyl] from comment #5)
> Comment on attachment 8359099 [details] [review]
> GH Pull Request
> 
> Looks like the code needs to be updated to bring the latest changes.
> In 'shared/resources/keyboard_layouts.json', we have changed the format from
> "appOrigin" to "appManifestURL".

Done, I just updated https://github.com/mozilla-b2g/gaia/pull/14142 https://github.com/laukstein/gaia/commit/c7064fe1dc701bd6c51d063ef0c7b024a849754e

Comment 7

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8359099 [details] [review]
GH Pull Request

Please address the following:

* Rebase against master. The PR cannot be merged and the lv.dict file is missing. [1]
* Squash all commits into one, and name the commit "Bug 958919 - Add Latvian keyboard" (or something more elaborate). [1]
* Switching to numbers does not work (click on ?123), because if you want a different alternate layout you'll need to provide a full layout, not just the changes. You can put the alt keys in the general alt section instead.
* There are only alternatives for the Latvian characters, but not f.e. for ë or any other variation. Are you sure you only need the Latvian ones?

[1] This doc: https://gist.github.com/janjongboom/7194742 has info on how to squash and rebase commits.

If you have looked into this, please re-set the review flag.

Comment 8

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8359099 [details] [review]
GH Pull Request

Also please remove the changes in locales/languages_all.json and shared/js/. This only applies for whole translations of the OS, not for keyboard layouts.

Comment 9

[Raivis Dejus]

I also confirm that this layout is correct.

Comment 10

[Binyamin]

Comment on attachment 8359099 [details] [review]
GH Pull Request

>https://github.com/mozilla-b2g/gaia/pull/15536

Comment 11

[Binyamin]

(In reply to Jan Jongboom [:janjongboom] from comment #7)
> * Rebase against master. The PR cannot be merged and the lv.dict file is
> missing. [1]
> * Squash all commits into one, and name the commit "Bug 958919 - Add Latvian
> keyboard" (or something more elaborate). [1]
> * Switching to numbers does not work (click on ?123), because if you want a
> different alternate layout you'll need to provide a full layout, not just
> the changes. You can put the alt keys in the general alt section instead.
> * There are only alternatives for the Latvian characters, but not f.e. for ë
> or any other variation. Are you sure you only need the Latvian ones?

Jan, I just created a new pull request with a better compatibility.
* lv.dict you can find in https://github.com/mozilla-b2g/gaia/blob/master/keyboard/dictionaries/lv.dict
* I find squash too complicated and committed a new push request https://github.com/mozilla-b2g/gaia/pull/15536
* https://github.com/laukstein/gaia/commit/ec2b52df2bb2825c2281709d5c0f9eaf1fdaf0c2 line 47, ratio: 8 I replaced to 9. Hope it will solve the number keyboard issue.
* All Latvian related characters has been added in lv.js, there are no need to add "out of context" characters like "ë".

Comment 12

[Binyamin]

Attachment: ALT-tag-proposal.pdf

In order to improve the keyboard UX for languages like Latvian (where the maximum ALT alternative is one letter), we need to add ALT in order to show-up the alternative tags (avoid there long-tap bubble for alternative tags) - the same conception used on Android. The same conception for Latvian language has been used on Desktop for the beginning.

Comment 13

[Binyamin]

Even better proposal:
It could be even better prototype for "ALT-tag-proposal.pdf", if ALT tag would be applied on long touch. It would simplify the UX. It would apply with one simple stem, instead of two current keyboard 2 steps prototype or ALT-tag-proposal.pdf alternative.

Comment 14

[Binyamin]

Any reason why https://github.com/mozilla-b2g/gaia/pull/15536 is not jet pushed?

Comment 15

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8359099 [details] [review]
GH Pull Request

Because you have to set the review flag again for me to see it ;-)

Comment 16

[Binyamin]

Comment on attachment 8359099 [details] [review]
GH Pull Request

>https://github.com/mozilla-b2g/gaia/pull/15536

Comment 17

[Binyamin]

(In reply to Jan Jongboom [:janjongboom] from comment #15)
> Comment on attachment 8359099 [details] [review]
> GH Pull Request
> 
> Because you have to set the review flag again for me to see it ;-)

I found no option to remove review flag from https://bugzilla.mozilla.org/attachment.cgi?id=8359099&action=edit, perhaps I have not bugzilla permissions to do that.

Comment 18

[David Bolter [:davidb] (NeedInfo me for attention)]

(I don't see anything here (Latvian specific) the accessibility team needs to review. Removing flag.)

Comment 19

[Binyamin]

ALT-tag-proposal.pdf already mentioned in Bug #908487 https://bug908487.bugzilla.mozilla.org/attachment.cgi?id=794321 page 2 (tag "ñ" - Extra key depending on selected language: "ç" for pt-bz and "'" for en. For Latvian expected "'").
Bug #908487 currently moved to bug #973517 v1.5 system features.

Comment 20

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch v2

Comment 21

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8385993 [details] [review]
Patch v2

Binyamin, please find my comments on GitHub. At the moment the bottom bar of the keyboard is too wide, and the problem with the alternative layout is not solved (clicking ?123 still fails).

* To fix bottom bar, change space layout width to 8.
* To fix the ?123 please remove the `alternateLayouts` section and move
      '€': '$',
      '"': '„”«»'
into the `alt` section.

Also please test the commit before pushing again. You can do so by downloading B2G desktop (from http://ftp.mozilla.org/pub/mozilla.org/b2g/nightly/latest-mozilla-central/). Then create a gaia profile from command line:

GAIA_KEYBOARD_LAYOUTS=en,lv make

Then start up B2G desktop from command line via:

c:\path-to-b2g-desktop\b2g-bin.exe -profile c:\path-to-gaia\profile -jsconsole

Now you can go into Settings -> Keyboard and select Latvian.

Also, I can understand that squashing can be seem difficult at first, but it makes keeping track of the patches a lot easier for the reviewers.

After changing this patch, please set the review flag to '?' and to me again instead of posting a comment, as reviewers won't be notified of new comments, only of flags.

If you have any questions please ping me on IRC (janjongboom on irc.mozilla.org, #gaia or #b2g).

Comment 22

[Binyamin]

Comment on attachment 8385993 [details] [review]
Patch v2

Done. You can apply the pull request.

Comment 23

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

Hi Binyamin,

I am sorry, but unfortunately this bug was being left out of the radar. Could you test the patch on the current keyboard app again so that we could make sure it's ready to be merged?

Please set the review flag again to me, :rudyl, or :janjongboom again once you have test it.

Comment 24

[JM Gailis [:jm-gailis]]

We need to reopen this bug and the merge request on Github, while the Latvian (lv) locale starts to became a reality (more to come in the repositories).

Comment 25

[Binyamin]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #23)
> Hi Binyamin,
> 
> I am sorry, but unfortunately this bug was being left out of the radar.
> Could you test the patch on the current keyboard app again so that we could
> make sure it's ready to be merged?
> 
> Please set the review flag again to me, :rudyl, or :janjongboom again once
> you have test it.

Where (in https://github.com/mozilla-b2g/gaia) I can see the example of latest version keyboard files (the recent syntax of it)?

Comment 26

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

(In reply to Binyamin from comment #25)
> Where (in https://github.com/mozilla-b2g/gaia) I can see the example of
> latest version keyboard files (the recent syntax of it)?

I would recommend

https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/layouts/gl.js

since this is the most recent one get merged.

Comment 27

[Binyamin]

Attachment: Latvian keyboard based on latest version syntax

Comment 28

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

I will verify this pull request tomorrow.

Comment 29

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

Comment on attachment 8515540 [details] [review]
Latvian keyboard based on latest version syntax

I have verified the keyboard definition by loading it into Gaia Keyboard Demo. I can't review the layout but I can verify the layout does not overflow, so this is good to land.

To prevent the patch being left out of the loop, please always make sure your patch is attend by someone by flagging someone with review? or needinfo? next time (Just like what I did to myself yesterday.)

Comment 30

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

master: https://github.com/mozilla-b2g/gaia/commit/754b277a8108b5bbf678fdfb49f25d86866bc637

Comment 31

[Binyamin]

Why Latvian keyboard still does not appear in Firefox OS latest Nightly image Settings > Keyboards > Select Keyboards list?

Comment 32

[Kosmodrey]

Binyamin, see bug #1050574

Comment 33

[JM Gailis [:jm-gailis]]

The Latvian keyboard is available and fully working on my device, the ZTE Open C.
I'm using a Nightly build from the Mozfr, the french speaking mozilla community.
Actually, I found no bugs, it's fully working.
Thanks for your work on this core component of the lv l10n of Firefox OS!

Comment 34

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

Comment 32 is correct.