Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat, UX] Pick keyboard layout from keyboard #5583

Merged
merged 30 commits into from Nov 18, 2019
Merged

Conversation

@yparitcher
Copy link
Contributor

yparitcher commented Nov 10, 2019

this is a first draft for fixing keyboard locale support
see #5569 (comment)

the idea is that when pressing the globe icon a popup comes up to chose an input language (the same choices that are in the keyboard layout menu)
i made some of the necessary changes.

things i need help with:

  1. making a popup (similar to the openwith dialog) to chose the language when pressing the globe key.
    i put in comments the radio table needed for the languages options but i have no clue how the widget/input ui works to be able to make a proper dialog out of it.

  2. fixing all the keyboards:

  • make the local characters default: move position 5-8 to 1-4
  • shift the Äéß from position 9-12 to 5-8
  • get rid of the english characters in other languages
@Frenzie Frenzie added this to the 2019.11 milestone Nov 10, 2019
@Frenzie Frenzie added the UX label Nov 10, 2019
@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Nov 10, 2019

make the local characters default: move position 5-8 to 1-4

Mhhh, that won't do just like that, or you have to fix all the keyboards :)
The 1st layout (except for your hebrew one) is usually the default the keyboard codes wanted. (In the french keyboard, I have some russian on that globe layout.)
We'd need to rethink all current keyboards files then if we go that way.

get rid of the english characters in other languages

I think english/ascii should be accessible directly from a toggle layout key in all languages, because that's just practical (filenames, ...)

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 10, 2019

Mhhh, that won't do just like that, or you have to fix all the keyboards :)

@yparitcher Indeed, it's not the fault of any other keyboard that the Hebrew keyboard was written one way or the other! There is no bug here whatsoever. If you want the Hebrew keyboard fixed, fix it. Other keyboards should not be broken and nothing should be shifted.

If I recall correctly you said you wrote it this way because it was most convenient for you, switching between English and Hebrew. That's fine, but if that's considered a bug it's exclusively a bug that you purposefully put into that specific keyboard.

I think english/ascii should be accessible directly from a toggle layout key in all languages, because that's just practical (filenames, ...)

@poire-z That's not really your or my business to decide but depends on the sensibilities and traditions of the keyboards in question. ;-) The whole point here is to make it unproblematic to quickly switch between layouts.

The most sensible way is to select up to four or five desired layouts that you can easily toggle between. Also see #5414 (comment)

Why five? Because then you can map 'em to swipes on the globe key. ;-) Plus you get a popup along these lines:

@Frenzie Frenzie modified the milestones: 2019.11, 2019.12 Nov 10, 2019
@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Nov 10, 2019

That's not really your or my business to decide but depends on the sensibilities and traditions of the keyboards in question. ;-)

Except keyboard traditions never had 12 layouts like we do :) So we can be creative and build up future traditions :)

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 10, 2019

I'd prefer to reduce it to 6 (?) tbh, getting rid of the globe layer entirely. It can simply be refactored into a separate Cyrillic keyboard.

Unfortunately I'm not fully up to snuff on Cyrillic, but at least unlike Arabic I can read most of the characters. :-P

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Nov 10, 2019

I guess I'd be ok with 6 (all these with some popups where appropriate):

  1. lowercase letters in the "locale" (or arabic, hebrew single-case letters, korean thingies)
  2. Shift > uppercase letters in the "locale" (or arabic, hebrew diacritics when no uppercase)
  3. Sym > numbers, arrow keys, punctuations, parenthesis... (the same for all keyboards?)
  4. Shift + Sym > 2nd layer of punctuation, parenthesis, with arrow keys too
  5. Äéß > alternative lowercase letters, direct accentuated ones, less common letters, or some other common foreign ones, like some greek - or english letters for arabic/hebrew/korean keyboards for quick access (so, the icon would show Aes :)
  6. Shift + Äéß > alternative uppercase letters, other stuff, stupid stuff like http:// .com...
  7. Globe > switch to another layout

And ideally, the activated Shift SymÄéß should be highlighted, so we know where we're at :) so we can quicky unactivate them to get back to the first layout (I sometimes spend too much time on getting back to the first one:)

(Pinging @WaseemAlkurdi who's on the arabic keyboard in #5569.)

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 10, 2019

lowercase letters in the "locale" (or arabic, hebrew single-case letters, korean thingies)

Definitely how I'd do it. Most already work that way regardless. I think Hebrew and the current Arabic PR are the exception rather than the norm. And Cyrillic is an exception too, of course.

numbers, arrow keys, punctuations, parenthesis... (the same for all keyboards?)

Not necessarily 100 % the same, but it should be pretty close for the most part. I can definitely imagine punctuation being laid out differently (like «» for French).

alternative lowercase letters, direct accentuated ones, less common letters, or some other common foreign ones, like some greek - or english letters for arabic/hebrew/korean keyboards for quick access (so, the icon would show Aes :)

Some layouts (like Chinese and/or Korean) got rid of that button btw.

@yparitcher

This comment has been minimized.

Copy link
Contributor Author

yparitcher commented Nov 10, 2019

the point of this PR is to decouple keyboard language from keyboard layout.

Mhhh, that won't do just like that, or you have to fix all the keyboards :)

@yparitcher Indeed, it's not the fault of any other keyboard that the Hebrew keyboard was written one way or the other! There is no bug here whatsoever. If you want the Hebrew keyboard fixed, fix it. Other keyboards should not be broken and nothing should be shifted.

If I recall correctly you said you wrote it this way because it was most convenient for you, switching between English and Hebrew. That's fine, but if that's considered a bug it's exclusively a bug that you purposefully put into that specific keyboard.

sorry i was refering to the hebrew and arabic, but as you guys said for french and english the Cyrillic chraracters should be moved into thier own keyboard.

the goal is each keyboard should have one language, however the file needs to be fixed

I guess I'd be ok with 6 (all these with some popups where appropriate):

1. lowercase letters in the "locale" (or arabic, hebrew single-case letters, korean thingies)

2. Shift > uppercase letters in the "locale" (or arabic, hebrew diacritics when no uppercase)

3. Sym > numbers, arrow keys, punctuations, parenthesis... (the same for all keyboards?)

4. Shift + Sym > 2nd layer of punctuation, parenthesis, with arrow keys too

5. Äéß > alternative lowercase letters, direct accentuated ones, less common letters, or some other common foreign ones, like some greek - or english letters for arabic/hebrew/korean keyboards for quick access (so, the icon would show Aes :)

6. Shift + Äéß > alternative uppercase letters, other stuff, stupid stuff like http:// .com...

7. Globe > switch to another layout

And ideally, the activated Shift SymÄéß should be highlighted, so we know where we're at :) so we can quicky unactivate them to get back to the first layout (I sometimes spend too much time on getting back to the first one:)

this is the point however Globe > is not switching to a layout rather to a different language.
each language has up to 8 layouts, instead of the 12 we have now.

this is necessary in order to scale keyboard languages

The most sensible way is to select up to four or five desired layouts that you can easily toggle between. Also see #5414 (comment)

Why five? Because then you can map 'em to swipes on the globe key. ;-) Plus you get a popup along these lines:

this is a good idea to implement, however it would require new menu setting to select which are enabled, also to ensure that the current is enabled. and would require reworking the whole lang to keyboard table to track what the current layout is.

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 10, 2019

the point of this PR is to decouple keyboard language from keyboard layout.

For terminological purposes I think a term like "keyboard language" should be torpedoed in the bow before it can sail on. ;-) A full-on Dutch keyboard would refer either to the Dutch QWERTY layout (effectively replaced by US QWERTY since the 2000s) or Dutch/Belgian AZERTY (still a thing). There may also have been Dutch QWERTZ, or in any case QWERTZ typewriters were around in the Netherlands.

The tl;dr is that there are at least three distinct keyboard layouts associated with the Dutch language, probably four. English has at least two (US & UK). More for Dutch and English alike if you also count ANSI vs ISO. (I can't stand ISO.) The fact that the current two main keyboards for Dutch are currently referred to as "English" and "Français" in the KOReader interface is mildly unfortunate. KDE presents it like this, but Windows and Mac are very similar:

Screenshot_20191110_213754

A "locale" is a set of properties typically associated with a physical locale. Netherlands = € 1.000,00 with a US English International keyboard layout.

A keyboard layout can be part of a locale, but to refer to a keyboard layout as a keyboard locale is just confusing at best because it implies all that other baggage.

The code currently says something like setLayout. If that's confusing, please change it to setLayer or setLevel.

One layout has multiple layers, also known as levels. The base layer, the shift layer, and various other layers. In KOReader it's called the symbol layer, on a physical keyboard the Alt Gr. layer or the third layer (Shift being the second layer and Shift+Alt Gr. the fourth layer).

With all that out of the way, I don't really know what you mean by decoupling the keyboard language from its layout. I decoupled the UI language from the keyboard layout somewhat recently (Device → Keyboard layout), and the way I see this PR is just to make that slightly easier to put into effect, i.e., without closing the keyboard and going into a menu. The code so far is fully in line with that.

@yparitcher

This comment has been minimized.

Copy link
Contributor Author

yparitcher commented Nov 10, 2019

Now that that is out of the way,
can someone help me make a widget to chose the layout, and or help implement @Frenzie's idea

The most sensible way is to select up to four or five desired layouts that you can easily toggle between. Also see #5414 (comment)
Why five? Because then you can map 'em to swipes on the globe key. ;-) Plus you get a popup along these lines:

this is a good idea to implement, however it would require new menu setting to select which are enabled, also to ensure that the current is enabled. and would require reworking the whole lang to keyboard table to track what the current layout is.

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Nov 10, 2019

can someone help me make a widget to chose the layout, and or help implement @Frenzie's idea

You could get both :) by starting with the existing keyboard popup widget (re-using/extending/duplicating it), and you may quickly get something like that:

image

Then, may be instead of having a grid of squares, just have stacked rectangles with the full language name.
But I'd be fine with these 2 letters squares, as they would allow at least 5 swipes to quickly switch between them - and eventually add something so the most recently used are to be in these 5 squares, or allow organizing and ordering them into these 5 squares with our SortWidget.

(The above screenshot was just a fake by modifying frontend/ui/data/keyboardlayouts/keypopup/en_popup.lua with:

    _z_ = {
        "en",
        north = "he",
        northeast = "ar",
        northwest = "ko",
        east = "fr",
        west = "zh",
    },
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 10, 2019

The current menu could activate multiple layouts easily with only a minor adjustment. I'd start with that and the actual layout toggling (by tapping the globe key).

The swipe/popup thing is more of a later refinement/addition, where instead of having to tap two or three times you can just swipe in a particular direction.

Frenzie added a commit to Frenzie/koreader that referenced this pull request Nov 11, 2019
Required for <koreader#5583>.
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 11, 2019

@yparitcher Alright, with 2019.11 out the door I can now focus on this. #5588 implements the relevant popup functionality.

Frenzie added a commit that referenced this pull request Nov 11, 2019
Required for <#5583>.
yparitcher added 6 commits Nov 10, 2019
@yparitcher

This comment has been minimized.

Copy link
Contributor Author

yparitcher commented Nov 11, 2019

@yparitcher Alright, with 2019.11 out the door I can now focus on this. #5588 implements the relevant popup functionality.

thanks, but i don't think that is going to be enough, as that allows for a callback but does not yet allow dynamically assigning the key label this also has to be customizeable

local virtual_key = VirtualKey:new{
key = v,
label = v,
keyboard = parent_key.keyboard,
key_chars = key_chars,
width = parent_key.width,
height = parent_key.height,
}

unless i am understanding this wrong

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 12, 2019

I can probably add that tomorrow, but I don't think it's related to the few lines you quoted.

I'd add a VirtualKey:updateText() method (or some such) which you would then call using parent_key:updateText(new_lang) in the callback.

Alternatively it could be made slightly more complex with something like a label_func, in which case a simple update_keyboard() would suffice. Probably not really worth it for such a one off thing.

What exactly are you envisioning? Text next to the globe? Instead of the globe?

@yparitcher

This comment has been minimized.

Copy link
Contributor Author

yparitcher commented Nov 12, 2019

the popup keys have to be dynamically assigned to a language so the label on them matches to the language set and to the enabled languages. also the globe should probably change to the current layout once the popup appears.

@WaseemAlkurdi

This comment has been minimized.

Copy link
Contributor

WaseemAlkurdi commented Nov 14, 2019

Forgive me if I misunderstood the way this implementation works, but if we implement multiple keyboards this way, don't we have to remove the Latin character layout in each and every keyboard?

@yparitcher

This comment has been minimized.

Copy link
Contributor Author

yparitcher commented Nov 14, 2019

Forgive me if I misunderstood the way this implementation works, but if we implement multiple keyboards this way, don't we have to remove the Latin character layout in each and every keyboard?

yes that is the plan, they should only be needed in the latin based languages.

@Frenzie Frenzie changed the title keyboard locales [feat, UX] Pick keyboard layout from keyboard Nov 15, 2019
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 15, 2019

The 🌐 works fine btw (here instead of Sym, left of the current icon for easy contrast):

Screenshot_2019-11-15_14-56-14

The ⇧ is less obviously good; there might be other options too if you look in Unicode arrows.

Screenshot_2019-11-15_14-58-16

@NiLuJe

This comment has been minimized.

Copy link
Member

NiLuJe commented Nov 17, 2019

@Frenzie: Hmm, U+21E7 looks thicker in FreeSerif (at least in FF), but it's common enough that something higher in the list might be covering it...

While I'm here:

  • Nerdfont's shift is U+ED35 (caps is U+ED31)
  • Del is U+232B
  • Return is U+23CE (FreeSerif's one is weird, though). Or U+21B5 for CR. Or U+2BA0. Or U+EA10 (nerdfont).
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented on a0c31f6 Nov 17, 2019

Nice one :-)

yparitcher added 3 commits Nov 18, 2019
there are no conflicts so this is the simplest way
@yparitcher

This comment has been minimized.

Copy link
Contributor Author

yparitcher commented Nov 18, 2019

everything should be finished on my end, all that is needed to to consolidate each keyboard layout to 8 layers with one set of characters and get rid of the unnecessary ones.

Reader_2019-Nov-17_220613

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 18, 2019

Alright, I'll give it a look later. 👍

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 18, 2019

When no layouts are enabled:

./luajit: ./ffi/util.lua:520: bad argument #1 to 'pairs' (table expected, got nil)
stack traceback:
        [C]: in function 'pairs'
        ./ffi/util.lua:520: in function '__genOrderedIndex'
        ./ffi/util.lua:535: in function '(for generator)'
        frontend/ui/widget/virtualkeyboard.lua:62: in function 'callback'
        frontend/ui/widget/virtualkeyboard.lua:294: in function 'handleEvent'
        frontend/ui/widget/container/inputcontainer.lua:259: in function 'handleEvent'
        frontend/ui/widget/container/widgetcontainer.lua:88: in function 'propagateEvent'
        frontend/ui/widget/container/widgetcontainer.lua:106: in function 'handleEvent'
        frontend/ui/widget/container/widgetcontainer.lua:88: in function 'propagateEvent'
        frontend/ui/widget/container/widgetcontainer.lua:106: in function 'handleEvent'
        frontend/ui/widget/container/widgetcontainer.lua:88: in function 'propagateEvent'
        ...
        frontend/ui/widget/container/widgetcontainer.lua:106: in function 'handleEvent'
        frontend/ui/widget/container/widgetcontainer.lua:88: in function 'propagateEvent'
        frontend/ui/widget/container/widgetcontainer.lua:106: in function 'handleEvent'
        frontend/ui/uimanager.lua:674: in function 'sendEvent'
        frontend/ui/uimanager.lua:46: in function '__default__'
        frontend/ui/uimanager.lua:1014: in function 'handleInputEvent'
        frontend/ui/uimanager.lua:1075: in function 'handleInput'
        frontend/ui/uimanager.lua:1119: in function 'run'
        ./reader.lua:260: in main chunk
        [C]: at 0x55d4de3fd77e
Copy link
Member

Frenzie left a comment

lgtm, besides that one tiny issue

fix
@Frenzie Frenzie added the enhancement label Nov 18, 2019
@Frenzie Frenzie merged commit 5e8d122 into koreader:master Nov 18, 2019
1 check passed
1 check passed
ci/circleci: build Your tests passed on CircleCI!
Details
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Nov 18, 2019

Thanks for putting in the effort! 👍

Frenzie added a commit to Frenzie/koreader that referenced this pull request Nov 18, 2019
Follow-up to koreader#5583.
@yparitcher yparitcher deleted the yparitcher:Gkeyboard branch Nov 18, 2019
Frenzie added a commit that referenced this pull request Nov 18, 2019
Follow-up to #5583.

* Add separate Cyrillic/Russian keyboard

* And remove that from English

* Update French keyboard (is easy)

* Remove yahzhert, that was just a double of QWERTY

* Update Spanish keyboard

* Reduce Japanese to 4 layers and add globe

* Reduce Korean to 4 layers

* Reduce Greek to 8 layers
@Frenzie Frenzie mentioned this pull request Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.