Handwriting icon for swiching from symbol panel back to handwriting page

RESOLVED WONTFIX

Status

Firefox OS
Gaia::Keyboard
RESOLVED WONTFIX
3 years ago
2 months ago

People

(Reporter: timdream, Assigned: Lavar Askew)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor-lang=zh][lang=js])

Attachments

(1 attachment)

See bug 796486 comment 11. We would need an icon to replace the [ABC] label there.
Flags: needinfo?(ofeng)
Hi Carol, as discussion offline, need your help here.
Flags: needinfo?(ofeng) → needinfo?(chuang)

Comment 2

3 years ago
Created attachment 8569035 [details]
Keyboard_handwriting_icons 0225.zip

Hi,
The attachment is the handwriting icon images. Please help update it, thanks!
Flags: needinfo?(chuang) → needinfo?(timdream)
Thanks! Leaving this bug for contributor to grab.
Flags: needinfo?(timdream)
I want to fix this bug . Need details :)
Flags: needinfo?(timdream)
Hi Atique,

Please read the cloned bug. This bug is about changing the label of the switch-back-to-handwriting-pad button to the attached image. You can see the example with Firefox here:

http://timdream.org/gaia-keyboard-demo/#zh-Hans-Handwriting 

Press the [12&] here you will find the [ABC] button to replace.

Now, this is a bit tricky. Since we are replacing that with an image, the patch will not be as simple as changing the text in the JSON file. I am thinking that the way to do it should be adding className to the button definition and to show a background image with that definition from style sheet. There are other button do that as well.

You can find the code at
https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/layouts/zh-Hans-Handwriting.js

Please read
https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia

To understand how to build and run Gaia on your computer, preferably on the new Mulet runtime.
Assignee: nobody → softfilebd
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
BTW, please run ./tools/png_recompress.sh against images so that we can keep the build size small.
Unassign due to inactivity.
Assignee: softfilebd → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 8

3 years ago
Tim, I'd like to take this bug as it seems it is still available.  I will follow your details shown above.
Assigned!
Assignee: nobody → open.hyperion
Status: NEW → ASSIGNED
(Assignee)

Comment 10

3 years ago
Tim,

does this fix only need to be implemented for zh-Hans-Handwriting.js?  Based on the demo above I imagine that we're trying to interpret hand written Chinese characters into the closest Chinese glyph.  Does this feature need to be implemented for all other languages as well?
Flags: needinfo?(timdream)
No, we don't need to put this image anywhere other than zh-Hans-Handwriting layout at the moment.
Flags: needinfo?(timdream)
(Assignee)

Comment 12

3 years ago
Hi Tim,

would you happen to know how I add the zh-Hans-Handwriting keyboard layout to the list of choosable keyboard layouts?

I've been trying to learn how the keyboard button that allows you to change the keyboard layout/language works, but on second thought I think I may be in the wrong place.  Maybe I need to be looking in the settings code.

I will try that next.

Thanks for listening.

-Lavar
(In reply to Lavar Askew from comment #12)
> Hi Tim,
> 
> would you happen to know how I add the zh-Hans-Handwriting keyboard layout
> to the list of choosable keyboard layouts?
> 

You can do that by adding the layout here:

https://github.com/mozilla-b2g/gaia/blob/d7dfdab49ec23/Makefile#L440-L443

> I've been trying to learn how the keyboard button that allows you to change
> the keyboard layout/language works, but on second thought I think I may be
> in the wrong place.  Maybe I need to be looking in the settings code.
> 
> I will try that next.
> 
> Thanks for listening.
> 
> -Lavar
(Assignee)

Comment 14

3 years ago
I have mimicked the css code for .search-icon in keyboard.css by adding the following:

.handwriting-icon {
  background: url('images/icon_handwriting.png') left calc(50% - 0.1rem) bottom 1.2rem / 3rem no-repeat;
}

/* hide actual text for handwriting-icon */
.keyboard-key.handwriting-icon > .visual-wrapper > .key-element,
.keyboard-key.handwriting-icon.highlighted > .visual-wrapper > .key-element {
  color: transparent;
}

The problem I am having now replacing the 'ABC' text with the .handwriting-icon css code.

I have tried editing the 4th row of the key 2-D array in Keyboards['zh-Hans-Handwriting'].  The handwriting icon shows up, but so does the 'ABC'.  Also the event to switch the keyboard back to handwriting mode only fire on the 'ABC'.

Even though I have tried to follow the search icon as an example in keyboard.css I do not see how to replace the 'ABC' value with the handwriting icon in the same way that the search icon replaces the '↵' value.

Any ideas?
(In reply to Lavar Askew from comment #14)
> I have tried editing the 4th row of the key 2-D array in
> Keyboards['zh-Hans-Handwriting'].  The handwriting icon shows up, but so
> does the 'ABC'.  Also the event to switch the keyboard back to handwriting
> mode only fire on the 'ABC'.
> 

Interesting. There is a property called |basicLayoutKey| allow you to replace the label, but I guess that won't work if you want to replace it with an empty string since it's a faluey value

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

A proper solution is to go into the keyboard base and make sure the code consider the empty value like change |if (layout.basicLayoutKey)| to  |if (typeof layout.basicLayoutKey === 'string)|.

I have not look into that part of keyboard code a long time ago maybe that's not the cause. I might be wrong there.. we could think of something else if so.

Thank you for investigating!
(Assignee)

Comment 16

3 years ago
Hey Tim,

I see what you're saying....

No need to set the value of basicLayoutKey to the empty string.  I just check the value of the input manager engine for the particular layout.  If it is handwriting then I set the className of the pageSwitchingKeyObject to 'handwriting-icon', which I have defined in keyboard.css.

Let me know if this was the proper approach.  You can see the code changes in my repo from the links below.

Thanks a lot for your help.

-Lavar

https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/4611fdabce772c608eda6645a1ad238758dc5dc2/Makefile#L442

https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/4611fdabce772c608eda6645a1ad238758dc5dc2/apps/keyboard/style/keyboard.css#L731-L739

https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/keyboard-branch/apps/keyboard/js/keyboard/layout_manager.js#L225-L229
I am sorry for the delayed reply. You are making good progress. Thanks.

(In reply to Lavar Askew from comment #16)
> Hey Tim,
> 
> I see what you're saying....
> 
> No need to set the value of basicLayoutKey to the empty string.  I just
> check the value of the input manager engine for the particular layout.  If
> it is handwriting then I set the className of the pageSwitchingKeyObject to
> 'handwriting-icon', which I have defined in keyboard.css.
> 
> Let me know if this was the proper approach.  You can see the code changes
> in my repo from the links below.
> 
> Thanks a lot for your help.
> 
> -Lavar
> 
> https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/
> 4611fdabce772c608eda6645a1ad238758dc5dc2/Makefile#L442

This should not be in the patch. Only for test locally.

> 
> https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/
> 4611fdabce772c608eda6645a1ad238758dc5dc2/apps/keyboard/style/keyboard.
> css#L731-L739

Image should be indeed set on from the CSS.

It is preferable to remove the label from basicLayoutKey instead of CSS, but I can live with the solution like this (except the selector, see below).

> 
> https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/
> keyboard-branch/apps/keyboard/js/keyboard/layout_manager.js#L225-L229

The imEngine name is set up the layout <div> I think. You don't need to write extra code to target that button.
(Assignee)

Comment 18

3 years ago
(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #17)
> I am sorry for the delayed reply. You are making good progress. Thanks.
> 
> (In reply to Lavar Askew from comment #16)
> > Hey Tim,
> > 
> > I see what you're saying....
> > 
> > No need to set the value of basicLayoutKey to the empty string.  I just
> > check the value of the input manager engine for the particular layout.  If
> > it is handwriting then I set the className of the pageSwitchingKeyObject to
> > 'handwriting-icon', which I have defined in keyboard.css.
> > 
> > Let me know if this was the proper approach.  You can see the code changes
> > in my repo from the links below.
> > 
> > Thanks a lot for your help.
> > 
> > -Lavar
> > 
> > https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/
> > 4611fdabce772c608eda6645a1ad238758dc5dc2/Makefile#L442
> 
> This should not be in the patch. Only for test locally.
> 
> > 
> > https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/
> > 4611fdabce772c608eda6645a1ad238758dc5dc2/apps/keyboard/style/keyboard.
> > css#L731-L739
> 
> Image should be indeed set on from the CSS.
> 
> It is preferable to remove the label from basicLayoutKey instead of CSS, but
> I can live with the solution like this (except the selector, see below).
> 
> > 
> > https://github.com/open-hyperion/zh-Hans-Handwriting-keyboard-branch/blob/
> > keyboard-branch/apps/keyboard/js/keyboard/layout_manager.js#L225-L229
> 
> The imEngine name is set up the layout <div> I think. You don't need to
> write extra code to target that button.

I am not sure what you mean here.  I commented out the code in layout_manager.js that explicitly targets that button, but then I do not understand how to set the class of a button that needs the handwriting icon rather than the 'ABC' text.

I tried entering { value: '<div class="handwriting-icon"/>'} into the keys object, but that does not work.  It only adds another button to the row in zh-Hans-Handwriting.js.

I do not have a lot of experience with CSS so maybe I'm just missing something obvious.  Can you provide another hint?
(In reply to Lavar Askew from comment #18)
> 
> I am not sure what you mean here.  I commented out the code in
> layout_manager.js that explicitly targets that button, but then I do not
> understand how to set the class of a button that needs the handwriting icon
> rather than the 'ABC' text.
> 
> I tried entering { value: '<div class="handwriting-icon"/>'} into the keys
> object, but that does not work.  It only adds another button to the row in
> zh-Hans-Handwriting.js.
> 
> I do not have a lot of experience with CSS so maybe I'm just missing
> something obvious.  Can you provide another hint?

I am very sorry for the confusion. Upon inspecting Keyboard app DOM with WebIDE, I've realized I am talking about a className have since removed. So you indeed need to specify a className on the switch button in order to target it with CSS.

Depend on your availability of working on this bug further, I am more leaning toward a general solution for layouts to set the styles of the switching key, rather than another special |if ( ... )| block in the main keyboard code.

Currently, layout can already specify the label with |basicLayoutKey|. We should generalize that by offer the layout JS to specify a |switchingLayout: { ... }| and use the properties (value/className etc.) to set up our button.

That's definitely more work than what's planned here though. If you don't want to do that, we could definitely land a |if ( ... )| block first and offer a generic solution in another bug.

Thanks!
(Assignee)

Comment 20

3 years ago
Sure, I'm interested in doing the work.  Can we just just close out this bug with the |if ( ... )| patch and then continue with a new bug like you mentioned?

Also I will start using WebIDE.  Thanks for mentioning what you're using.  I have been using mulet, but when I try to set breakpoints and step through the code the UI becomes unresponsive.  So basically I've just been reading the code and using your hints to get me through this bug.  It's been working out so far, but I could definitely adopt a better process.
(In reply to Lavar Askew from comment #20)
> Sure, I'm interested in doing the work.  Can we just just close out this bug
> with the |if ( ... )| patch and then continue with a new bug like you
> mentioned?

No problem. Shoot me an r? flag and I will review it.

> Also I will start using WebIDE.  Thanks for mentioning what you're using.  I
> have been using mulet, but when I try to set breakpoints and step through
> the code the UI becomes unresponsive.  So basically I've just been reading
> the code and using your hints to get me through this bug.  It's been working
> out so far, but I could definitely adopt a better process.

Mulet shouldn't be a lot different from WebIDE. It comes with DOM inspector too.

My development flow only works if there is a device available.
Mentor: timdream

Comment 22

2 months ago
Firefox OS is not being worked on
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.