Closed Bug 1011482 Opened 6 years ago Closed 3 years ago

[keyboard] need value and compositeKey fields for alt keys

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: raniere, Assigned: raniere, Mentored)

References

Details

Attachments

(1 file)

46 bytes, text/x-github-pull-request
salva
: feedback+
Details | Review
Right now the alt keys supports:

(a) single characters:

~~~
'a': 'áãàâ'
~~~

(b) compositeKey:

~~~
'$': '€ £ R$'
~~~

I need the alt keys to also support value with compositeKey:

~~~
'$': [{value: '€', compositeKey: '\\Euro'}, {value: '£', compositeKey: '\\textsterling'}, {value: 'R$', compositeKey: 'R$'}]
~~~
Blocks: 1004057
Hi Raniere, Salva,

With the discussion on the mailing list, I think the requirement is,
For each alternative key/compositeKey, we would define both its presentation and the exact chars we need to send:
 
I would suggest we do this with,
{
  label: '€',         // presentation text for this alternative
  value: '\\Euro'     // chars we send
}
Just for clarification, let's provide a better use case:

To allow specialized layouts for mark-up languages, we need to provide key choices able to send more than one character, a.k.a, compositeKey.

For instance, in a LaTeX mathematical layout, the users could find the integral symbol ∫ which must send \int and long pressing, the users should be able to find ∮ ∲ ∯ or ∰ that must send \oint, \varointclockwise, \oiint and \oiiint. Thus, an extension of choice syntax is required to separate key label from value sent.
What is the best way to provide a "demo" for this patch?
Attachment #8436244 - Flags: review?(salva)
Attachment #8436244 - Flags: review?(rlu)
Flags: needinfo?(salva)
Your own develop. Once this land and you will use it in your project, update the bug and use your project to show the applications!
Flags: needinfo?(salva)
Comment on attachment 8436244 [details] [review]
add-label-for-alternatives

I give you my feedback+ as I can not review the patch. Take into account my comments on GitHub.
Attachment #8436244 - Flags: review?(salva) → feedback+
Best way to test this, is to add a new test case in the uitest application. Can be verified by hand or through automated script then.
Comment on attachment 8436244 [details] [review]
add-label-for-alternatives

Sorry that I did not make this clear earlier, but I would like to encourage you to write a separate keyboard app for the GSoC project if these changes are specifically for the Math keyboard.

This would have 2 benefits,
 1. You don't have to consider if the use cases are general enough to make these changes into the mainstream keyboard app.
 2. You don't have to be blocked by the review process of Gaia project.
    As you may already know, now we're quite busy with the v2.0 development process, so we may not respond to these reviews timely.
    (I am really sorry for that.)

--
We have another student project working on Keyboard, and you may find his repo here, https://github.com/psbots/firefoxos.ime
As you can see, it is also a fork of our current built-in keyboard.

I'll clear the review first, and hope we can both benefit from this proposal.
Attachment #8436244 - Flags: review?(rlu)
Blocks: 1027646
Hi Tim,

at Bug 1029356 you starting use a Array to store the alternatives keys instead of a string. Could we use the same structure of the normal key?
Flags: needinfo?(timdream)
(In reply to Raniere Silva from comment #8)
> Hi Tim,
> 
> at Bug 1029356 you starting use a Array to store the alternatives keys
> instead of a string. Could we use the same structure of the normal key?

I don't understand you question: could you be more specific on how you want to use the array?
Flags: needinfo?(timdream)
Oh, I get what you meant -- yes, you should definitely make the normalized form of each alternative keys likes the normal keys, on top of bug 1029356.

Assigning to you since you are actively work on this.
Assignee: nobody → ra092767
Mentor: rlu, timdream
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Depends on: 1029356
Depends on: 1037942
No longer depends on: 1029356
Tim, could you take a look at my patch?
Flags: needinfo?(timdream)
Your patch is working toward the right direction. Good job! Make sure the original tests passes and the new tests are added.

I was thinking about removing the different alternatives for each case form, e.g., from

alt: { 'a': [ { value: 'á' }, ... ],
       'A': [ { value: 'Á' }, ... ] },

to

alt: { 'a': [ { value: 'á' }, ... ] },

and have render.js figure out the actual upper label/keycode of |{ value: 'á' }| on runtime just like other keys, but it might be too much of the change for this bug.
Flags: needinfo?(timdream)
Raniere,

Quick question -- the purpose of this bug is to have alt char menu supporting composition keys (i.e. keys with more than one character), but it looks like we already have that support and used in es.js

https://github.com/mozilla-b2g/gaia/blob/3aa82ecc1fb1a1b410cb24b0f992cf80cfe85b33/apps/keyboard/js/layouts/es.js#L16

Does that already fit your use case? 

(I am not saying you should stop working on this bug -- just that if this bug blocks your work and the above usage solve your problem, you should be able to do your work first w/o this bug)
Flags: needinfo?(ra092767)
Tim,

> Quick question -- the purpose of this bug is to have alt char menu supporting composition keys (i.e. keys with more than one character), but it looks like we already have that support and used in es.js

You are correct that it support composition keys. For my use case I need to show the use some unicode and insert it as a markup (see comment #1).
Flags: needinfo?(ra092767)
Make sense, I should have read the entire bug!
Raniere, are you still working on this or any of the bug taken? We would like to hear some updates here so features won't be blocked on bugs you are working on, thanks!
Flags: needinfo?(ra092767)
Tim, please reassign to yourself any bug that is blocking next release of Firefox OS.
Flags: needinfo?(ra092767)
Firefox OS is not being worked on
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.