Closed Bug 936840 Opened 8 years ago Closed 8 years ago

Bulgarian Keyboard Layouts

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: chilyashev, Assigned: chilyashev)

References

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.114 Safari/537.36

Steps to reproduce:

Hello,

As mentioned in bug 932606, I asked around the Bulgarian community and we agreed that it's best to have the three layouts included.

I've committed them to my fork of Gaia, but haven't sent a pull request yet.
The commit can be found here - https://github.com/chilyashev/gaia/commit/439368e5d341996d3e5cb84c397c8eedbe58ea48

I hope everything's fine.
Flags: needinfo?(timdream)
Great! Do flag me or Rudy for review when you think this is ready.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(timdream)
Attachment #830167 - Flags: review?(timdream)
Comment on attachment 830167 [details] [review]
The pull request for adding the Bulgarian keyboard layouts.

Please remove/replace the layout added in bug 932606 and re-request for review again.

Rudy, should these layouts of the same language contained in the same file or it should be put separately?
Attachment #830167 - Flags: review?(timdream) → feedback?(rlu)
Comment on attachment 830167 [details] [review]
The pull request for adding the Bulgarian keyboard layouts.

The unnecessary bg.js file is removed. 
Commit - https://github.com/chilyashev/gaia/commit/319e7515090354d2a095203354328454fb3a95f2
Re-requesting review.
Attachment #830167 - Flags: review?(rlu)
Comment on attachment 830167 [details] [review]
The pull request for adding the Bulgarian keyboard layouts.

It is good put each of these layouts into separate files.

Please help also add an entry to this file,
https://github.com/mozilla-b2g/gaia/blob/master/shared/resources/keyboard_layouts.json

It should look like,
 "bg": [
        {"layoutId": "bg-BDS", "appOrigin": "keyboard.gaiamobile.org"},
        ...
        ...
    ]


BTW, any chance that we could cut down the menu label? It is a little too long to be shown completely in settings menu and the layout menu when you long press the global key on the keyboard.

--
Thanks.
Please re-flag me as reviewer when this is ready.
Attachment #830167 - Flags: review?(rlu)
Attachment #830167 - Flags: feedback?(rlu)
Attachment #830167 - Flags: feedback+
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #3)
> Rudy, should these layouts of the same language contained in the same file
> or it should be put separately?

Rudy, does that means for building a Bulgarian Fx phone, one should specify all three layouts in the build config separately?
Flags: needinfo?(rlu)
Yes, as you said, it would need to specify the flag as follows per current design,
GAIA_KEYBOARD_LAYOUTS=en,pt-BR,es,de,fr,pl,bg-BDS,bg-Pho-Ban,bg-Pho-Trad
Flags: needinfo?(rlu)
Comment on attachment 830167 [details] [review]
The pull request for adding the Bulgarian keyboard layouts.

Ok, I think I got it this time. 
I kinda messed up the commits, but it should be fine now.

I cut the layout names as much as I could, hoping it doesn't make them too confusing for the users.

The last commit - https://github.com/chilyashev/gaia/commit/56f3f8343979cb3aba2869b7981a8d6977ca9d9b
Attachment #830167 - Flags: review?(rlu)
Assignee: nobody → chilyashev
Status: NEW → ASSIGNED
Comment on attachment 830167 [details] [review]
The pull request for adding the Bulgarian keyboard layouts.

Looks good!

Can you help squash all your commits into a single one?
That is our policy for better managing the codebase.

You would need to do,
|git rebase -i master|

Let me know if you need any help on that.
Attachment #830167 - Flags: review?(rlu) → review+
Ok, I think I managed to squash them.

Like I mentioned, I messed up the previous commits' messages and had to do some not so smart things.

Could you please confirm that the final version of the pull request will do?
Comment on attachment 830167 [details] [review]
The pull request for adding the Bulgarian keyboard layouts.

Hi, is something wrong with the way I squashed the commits?

Or maybe I should have commented on the attachment when everything was done.
Attachment #830167 - Flags: feedback+ → feedback?(rlu)
(In reply to Mihail Chilyashev from comment #11)
> Comment on attachment 830167 [details] [review]
> The pull request for adding the Bulgarian keyboard layouts.
> 
> Hi, is something wrong with the way I squashed the commits?
> 
> Or maybe I should have commented on the attachment when everything was done.

No, it is fine but for some reason (conflict) the patch cannot be automatically merged into master.
I'll take a look to rebase your change on master if you don't beat me.
Thanks.
Attached file Patch V1.1
Hi Mihail,

I've sent another pull request to include your change and also add "auto correction" to the other 2 layouts.
You may take a look at the 2nd commit.

If you think this is Ok, then I'm going to merge this.
Thank you.
Attachment #8337381 - Flags: review?(chilyashev)
Attachment #830167 - Flags: feedback?(rlu) → feedback+
(In reply to Rudy Lu [:rudyl] from comment #13)
> Created attachment 8337381 [details] [review]
> Patch V1.1
> 
> Hi Mihail,
> 
> I've sent another pull request to include your change and also add "auto
> correction" to the other 2 layouts.
> You may take a look at the 2nd commit.
> 
> If you think this is Ok, then I'm going to merge this.
> Thank you.

Hi,

I think it's OK to be merged.

Thank you.
Comment on attachment 8337381 [details] [review]
Patch V1.1

Hello again,

I can't edit the flags of this attachment, so I can't set it to review+
Like I said in the previous comment, it's OK.
Comment on attachment 8337381 [details] [review]
Patch V1.1

Setting the flag for you. r=chilyasev
Attachment #8337381 - Flags: review?(chilyashev) → review+
Merged to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/8ba958e7c498724ece8a97b0a78923100a32fd30

--
Mihail,

Thanks for the contribution.
Nice work!

Jan, thanks for the reminding.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Attachment #830167 - Attachment is obsolete: true
Hi,

There are some improvements to the layouts thanks to :stoyan.
I was going to just submit a pull request, but the contribution guidelines say to open a bug.
I wasn't sure if I should reopen the bug, so here I am :)
Flags: needinfo?(rlu)
Please help open another bug for the improvement that you're going to do.
Thanks.
Flags: needinfo?(rlu)
You need to log in before you can comment on or make changes to this bug.