[keyboard refactor] copy the settings panel of the current keyboard

RESOLVED FIXED

Status

Firefox OS
Gaia::Keyboard
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: djf, Assigned: djf)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
The refactored keyboard in test_apps/demo-keyboard does not have a settings panel. We can copy the settings panel of the existing keyboard.
(Assignee)

Comment 1

4 years ago
Carrie and Mike: 

Do you want a setting to control dynamic hit target resizing?  Should the user be able to turn it off?  Should the user be able to control how aggressive it is?

This bug is for the settings panel of the 1.4 (latin) keyboard app. If you want any controls other than those we have now, please let me know.
Blocks: 956169
Flags: needinfo?(mtsai)
Flags: needinfo?(cawang)
(Assignee)

Comment 2

4 years ago
Carrie and Mike: 

Also, I need a UX decision about case switching.  In 1.2 and 1.3 we display uppercase letters only. My refactored keyboard displays lowercase and uppercase.  Can we keep this behavior?  Should there be a setting so that we can be like apple or like android depending on the user's preference?

Comment 3

4 years ago
Created attachment 8355400 [details]
[1.4 Keyboard 4 inch] Layout & Behavior_Draft.pdf

Please refer to Page. 7 - Typing Hint
Flags: needinfo?(mtsai)

Comment 4

4 years ago
Hi David, Happy New Year! 
As to the case switching issue. We had done some study and had internal discussion(including Josh C.). UX suggests to have upper case letter for default due to better visual recognition/perception during typing. Also design wise, the upper case typography is better than lower case letter in appearance. 

To prevent from unclear typing case status, we design to let typing hint show exact input case status as you can see in page.7 of the draft spec. In default lower case mode, although the UI letters are always in upper case, the typing hint bubble will show lower case letter so the user clearly notes that he/she is typing lower case letters. This subtle but effective hint can be a good help. ( In the phone market, iPhone and Black Berry are only ones using upper case UI always, but even when in lower case mode, the typing hint bubble shows upper case letter, which can be confusing. )

And we also suggest not to have preference settings to keep it simple.
p.s. iPhone and Android phones done have this option in settings, neither.

Thanks.
I'd like to reconsider the only-uppercase keyboard; I get a bug report of it like twice per week of a user who thinks it's a bug.
(In reply to David Flanagan [:djf] from comment #1)
Hi David, 

We'd suggest not to provide the switch in settings. Since dynamic hit target is something seamless, users shouldn't need to adjust anything (and usually they don't). In addition, from our reference phones, some of them provide the level adjustment of auto-correction, but none of them expose any clues of DHT to users. Our goal here is to let users experience the improved accuracy, but we don't really need to let them know what we have done. :-)

Thanks!

> Carrie and Mike: 
> 
> Do you want a setting to control dynamic hit target resizing?  Should the
> user be able to turn it off?  Should the user be able to control how
> aggressive it is?
> 
> This bug is for the settings panel of the 1.4 (latin) keyboard app. If you
> want any controls other than those we have now, please let me know.
Flags: needinfo?(cawang)
(Assignee)

Updated

4 years ago
Assignee: nobody → dflanagan
(Assignee)

Comment 7

4 years ago
Created attachment 8369166 [details] [review]
link to patch on github

Jan,

This patch mostly just copies the settings panel html,js, and css code from the current keyboard to the new keyboard.  It also defines a new settings.js module based on settings utility code in the the old keyboard.js

Note that it doesn't hook the settings up to anything: just does a console.log when it gets initial values and when the settings change.
Attachment #8369166 - Flags: review?(janjongboom)
Comment on attachment 8369166 [details] [review]
link to patch on github

Works exactly the same as normal keyboard, and the same settings are changed.

I still wonder if we can't store this stuff self-contained, as system apps don't have to change these values, because the Settings page lives in the app itself (unlike other system apps). The reason why we still use mozSettings currently is because it was like that before, I think making a clean break from that would have a nicer codebase. But maybe that's nice for a follow up.

r=me.
Attachment #8369166 - Flags: review?(janjongboom) → review+
(Assignee)

Comment 9

4 years ago
Jan,

I absolutely agree that we should not be using mozSettings here, and 3rd party keyboards won't be able to do that. But time is short, so I copied code that I could copy.

The main stumbling block I see is how to communicate settings changes between the settings panel running in one process with the rest of the keyboard runnning in a separate process.  mozSettings has the observer API that makes this easy.  localStorage has change events, too but we're not allowed to use localStorage.  IndexedDB is the obvious choice for storing settings, but it does not have a way to be notified of db changes.  So if we used that, the keyboard would have to query the db to check for changes each time it became visible. 

I suppose we could probably use the inter-app communication API to have the app notify other instances of itself when settings changed. But I haven't used IAC before myself.

In any case, yes, I think this would be a good followup, and I'd be happy to have someone take that on...
Flags: needinfo?(janjongboom)
(Assignee)

Comment 10

4 years ago
Landed to master: https://github.com/mozilla-b2g/gaia/commit/622e3c0be9f1b0cef27ff4b5d4ba21206f791685
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Hmmm, I kinda figured that the settings panel would run in the same process as the keyboard. If that's not the case then that's a bit more complicated. Still something we'd need to solve tho for 3rd party keyboards.
Flags: needinfo?(janjongboom)
(In reply to David Flanagan [:djf] from comment #9)
> Jan,
> 
> I absolutely agree that we should not be using mozSettings here, and 3rd
> party keyboards won't be able to do that. But time is short, so I copied
> code that I could copy.
> 

We should probably do it in another bug, and make sure SettingsDB values got picked up during OTA update.
BTW, even if two frames running in the same process, they cannot communicate with each other without the |window| reference of the other frame. There are some hackery way to get it with window.open() through, but that's not something better if you think using mozSettings is a hack.

(shared worker maybe?)

Anyhow, all that can be done later, even after we ship.
You need to log in before you can comment on or make changes to this bug.