1019472 - [Keyboard] [ZhuYin] ZhuYin Keyboard should follow the recommendation

Status: VERIFIED FIXED

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

Description

[Omega Feng [:Omega] [:馮於懋]]

[Steps To Reproduce]
1. Switch to ZhuYin keyboard
 
[Actual Result]
It's different from UX spec.

[Expected Result]
It should follow the recommendation of UX sepc.

Device: Hamachi
OS Version: 2.0.0.0-prerelease
Platform Version: 32.0a1
Build Identifier: 20140528160204

Comment 1

[Omega Feng [:Omega] [:馮於懋]]

See 983043 for latest UX spec.

Comment 2

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

We have an updated visual spec for this, see  attachment 8437450 [details] in bug 983043.

Comment 3

[Omega Feng [:Omega] [:馮於懋]]

It's a major issue in new feature. Nominating as 2.0 blocking.
Zhuyin is the major IME for zh-TW market.
Chunghwa Telecom to support Firefox OS: http://focustaiwan.tw/news/ast/201406110014.aspx

Comment 4

[Jason Smith [:jsmith]]

Bruce - Can you make a product call on whether this is needed for 2.0 or not?

Comment 5

[Jason Smith [:jsmith]]

Going to followup offline on this.

Comment 6

[Stephany Wilkes]

I discussed this bug (and others like it) with Jason in IRC today. This looks like a 2.0 feature that was not correctly or fully implemented to spec, and should not ship as-is as a result (per Omega's comment #3 and the specs referenced in this bug). 

To that end, I am removing the blocking flag but adding the feature-b2g flag for 2.0, which it looks like this bug should have had as it was on the work list for 2.0. If this is not correct, let me know and we can discuss making it a blocking bug if it was not indeed a feature.

Comment 7

[howie [:howie]]

This is not committed in 2.0, removing feature-b2g tag.

Comment 8

[DongShengXue]

Attachment: Patch V1 - pull request 22812

apps/keyboard/js/imes/jszhuyin/jszhuyin.js:
1, Add a conditional processing to choose layout between AlternateLayout, SymbolLayout, Full-width, Half-width.

apps/keyboard/js/layouts/zh-Hant-Zhuyin.js:
1, Set disableAlternateLayout is true, because AlternateLayout is different between single IME and multiple IME.
2, Add 8 layouts for choose between AlternateLayout, SymbolLayout, Full-width, Half-width.

Attention: There are some different between single IME's layouts and multiple IME's layouts.

Hi Rudy, could you help review this? Thanks a lot.

Comment 9

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

Comment on attachment 8472059 [details] [review]
Patch V1 - pull request 22812

DongSheng,

Thanks for taking this!
Please help take a look at bug 1046061 that we have to refine the layout definition of pinyin, and I think we should do the same thing for zhuyin:
 1. You should define only 2 special panels and leverage the original "alternateLayout" and "symbolLayout".
    As this change shows, 
    https://github.com/mozilla-b2g/gaia/pull/22347/files#diff-28730955dc03ccb1c1f94d8309c1d993R24

 2. Then we could simplify the layout switching code in zhuyin itself,
    example:
    https://github.com/mozilla-b2g/gaia/commit/df462661aac633b20ba8c7310c55135a90e552e9#diff-6996e645180cb49058c4164c822405b5L618

--
Please be informed this is still temporary solution, the proper fix would be implemented with bug 1047837.

I'll clear the review flag for now, please flag me again with updates and/or ping me with questions.

Comment 10

[DongShengXue]

(In reply to Rudy Lu [:rudyl] from comment #9)
> Comment on attachment 8472059 [details] [review]
> Patch V1 - pull request 22812
> 

Rudy，

Thanks for your reply.

>  1. You should define only 2 special panels and leverage the original
> "alternateLayout" and "symbolLayout".

I think the original "alternateLayout" and "symbolLayout" can not follow the recommendation.
If we follow the jspinyin way, we could only have one "alternateLayout" and one "symbolLayout", but UX design requires two "alternateLayout"s and two "symbolLayout"s. one group of "alternateLayout" and "symbolLayout" for the case that Zhuyin keyboard is the only one enabled keyboard, while another group for the case that more than one keyboards are enabled.

>  2. Then we could simplify the layout switching code in zhuyin itself

The layout switching code in zhuyin right now(imes/jszhuyin/jszhuyin.js), so I am not clear what your mean

Comment 11

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

This would need bug 1022609 because Zhuyin is different from pinyin that it would change the comma key position.

Comment 13

[DongShengXue]

Attachment: Patch V2 - pull request 22812

1. Define only 2 special panels and leverage the original "alternateLayout" and "symbolLayout".
2. Depends on: 1022609

Comment 14

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

Comment on attachment 8472169 [details] [review]
Patch V2 - pull request 22812

This looks good to me, except some nits.

Also ask for Omega/Carol help to do UI review.
I think we would need to apply the visual design change for 5-row layout.

Comment 15

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

Yeah we need to get rid of the gradient glitch of Zhuyin layout ...

Comment 16

[DongShengXue]

(In reply to Rudy Lu [:rudyl] from comment #14)
> Comment on attachment 8472169 [details] [review]
> Patch V2 - pull request 22812
> 
> This looks good to me, except some nits.
> 
> Also ask for Omega/Carol help to do UI review.
> I think we would need to apply the visual design change for 5-row layout.

I fixed all nits, except "We should define '±' as alternative char for '+' here.". Because the recommendation not define '±' as alternative char for '+'.

Comment 17

[Omega Feng [:Omega] [:馮於懋]]

Comment on attachment 8472169 [details] [review]
Patch V2 - pull request 22812

The UX part is all good.
The only possible issue is that in default panel under single IME, the Enter key width is too large. However, it needs Carol to confirm.

Comment 18

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

(In reply to DongShengXue from comment #16)
> (In reply to Rudy Lu [:rudyl] from comment #14)
> > Comment on attachment 8472169 [details] [review]
> > Patch V2 - pull request 22812
> > 
> > This looks good to me, except some nits.
> > 
> > Also ask for Omega/Carol help to do UI review.
> > I think we would need to apply the visual design change for 5-row layout.
> 
> I fixed all nits, except "We should define '±' as alternative char for '+'
> here.". Because the recommendation not define '±' as alternative char for
> '+'.

Thanks.
For the record, Omega confirmed the behavior that we don't need alternative chars for symbol panels in Zhuyin.
https://github.com/mozilla-b2g/gaia/pull/22812#discussion_r16165798

Comment 19

[Carol Huang [:Carol]]

Hi DongShengXue,
I've looked at the patch and I would like to know the text size of "ㄅㄆㄇ" in order to adjust ZhuYin's UI spec.
Thanks!

Comment 20

[DongShengXue]

(In reply to Carol Huang [:Carol] from comment #19)
> Hi DongShengXue,
> I've looked at the patch and I would like to know the text size of "ㄅㄆㄇ" in
> order to adjust ZhuYin's UI spec.
> Thanks!

Hi Carol, the text size of "ㄅㄆㄇ" is width ratio, the ratio is 2:10.

Comment 21

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

Hi Carol, for each char in the default panel, such as 'ㄅ', 'ㄉ', the font size is 2.4 rem.
Thanks.

--
For the record,
As of [ㄅㄆㄇ], key to switch back to the default panel, the font size is 1.5 rem.

Comment 22

[Carol Huang [:Carol]]

@Rudy,
Thank you for the information.

@DongShengXue,
1. For each character in the default panel, 'ㄅ', 'ㄉ'..etc, please change the font size to 2rem.
(I'll revise the spec document once I see the size looks ok on device! thanks!)

2. Please adjust the 5 rows of keyboard color (from ‘top' to ‘bottom') :
#636e71
#5e696c
#5b6668
#566063
#525b5e
Please see the visual spec on bug 983043 for detail

let me know if you have any question:)

Comment 23

[Carol Huang [:Carol]]

Comment on attachment 8472169 [details] [review]
Patch V2 - pull request 22812

please see comment 22

Comment 25

[Yuan Xulei [:yxl] (Leave from May 18)]

DongShengXue is taking paid time off and not available until Aug 25.
He will continue with this bug when he returns. If there is anything 
urgent to do with this bug, please ping me.

Comment 26

[DongShengXue]

(In reply to Carol Huang [:Carol] from comment #22)
> @Rudy,
> Thank you for the information.
> 
> @DongShengXue,
> 1. For each character in the default panel, 'ㄅ', 'ㄉ'..etc, please change the
> font size to 2rem.
> (I'll revise the spec document once I see the size looks ok on device!
> thanks!)
> 
> 2. Please adjust the 5 rows of keyboard color (from ‘top' to ‘bottom') :
> #636e71
> #5e696c
> #5b6668
> #566063
> #525b5e
> Please see the visual spec on bug 983043 for detail
> 
> let me know if you have any question:)

Hi Carol, I have changed the font size to 2rem and modified the row background color. Please Rudy help to review. Thanks.

Comment 27

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

Attachment: zhuyin_bottom_row.png

This update looks good to me except that we're missing the "right border" in the bottom row.

Carol, could you please help advise on the color we should use for the right border?
Thanks.

Comment 28

[Carol Huang [:Carol]]

Attachment: Bug 1019472_ZhuYin Keyboard.pdf

Hi DongShengXue,
Here's some comments for your patch.

1. The bottom row is missing dividers. (color#434b4d)
2. the 'back space' and 'enter' should keep the same size.
3. shorten the character's(ㄅㄆㄇ) distance

Please see the attachment for more detail. Thanks!!

Comment 29

[Carol Huang [:Carol]]

Comment on attachment 8478119 [details]
zhuyin_bottom_row.png

please see comment 28! thanks

Comment 30

[DongShengXue]

(In reply to Carol Huang [:Carol] from comment #28)
> Created attachment 8478142 [details]
> Bug 1019472_ZhuYin Keyboard.pdf
> 
> Hi DongShengXue,
> Here's some comments for your patch.
> 
> 1. The bottom row is missing dividers. (color#434b4d)
fixed
> 2. the 'back space' and 'enter' should keep the same size.
fixed
> 3. shorten the character's(ㄅㄆㄇ) distance
Modify to -0.3rem, please help to check

Comment 31

[DongShengXue]

Hi Carol, a digital error：
> > 3. shorten the character's(ㄅㄆㄇ) distance
> Modify to -0.1rem, please help to check

Comment 32

[Carol Huang [:Carol]]

Hi DongShengXue,
ZhuYin Keyboard looks good overall!
Thank you!!

But there's still few style issues that we need to fix.
let's file bugs for the graphic issue and close this bug first.

Rudy, 
how do you think?

Comment 33

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

As I commented with https://github.com/mozilla-b2g/gaia/pull/22812/files#r16700270

DongSheng, please remove that part of code (L744 - L758) and I'll adjust the style for Carol to review.
Thank you and sorry for the inconvenience.

Comment 34

[DongShengXue]

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

Got it.

Comment 35

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

I have amended the patch with some tweaks around the font size and show the result to Carol.

DongSheng,

I've checked with Carol that she's fine with our current implementation, so we don't open other follow-up bugs.
Thanks for your effort on this work!


--
Landed to Gaia,
https://github.com/mozilla-b2g/gaia/commit/bc9a482a98e16f011c82f40e3c42bababb159bf1

Comment 36

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

Attachment: Patch to add some style fixes.

For the record, attach the new pr.

Comment 37

[Lancy(Leave from Mozilla)]

Attachment: Veirfy_1019472_1.png

According to comment 30, in the new Flame 2.1 version, the 'back space' and 'enter' keys are not the same size and I am not sure the distance of the character's(ㄅㄆㄇ) is correct.

Hi Omega,
Could you please help to confirm the issue has been fixed?
Please refer to the attachmenets: Veirfy_1019472_1.png and Veirfy_1019472_2.png.

Comment 38

[Lancy(Leave from Mozilla)]

Attachment: Veirfy_1019472_2.png

Comment 39

[Omega Feng [:Omega] [:馮於懋]]

@Carol, need your help to confirm this.

Comment 40

[Carol Huang [:Carol]]

Hi,
The alignment looks good now, thanks!