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

[UX] Keyboard character selection popup #4886

Merged
merged 13 commits into from Apr 9, 2019
@@ -66,6 +66,158 @@ function VirtualKey:init()
self.callback = function() self.keyboard:downLine() end
else
self.callback = function () self.keyboard:addChar(self.key) end
self.hold_callback = function()
if not self.key_chars then return end

local popup_focus_manager = FocusManager:new{

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

@poire-z I started out writing this as a small(ish) inline function, but I figure I should separate it out into a VirtualKeyPopup (or some such).

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 9, 2019

Contributor

Yes, just to avoid creating and garbage collecting these few methods functions (even if it's little compared to the other stuff created in there :)

modal = true,
disable_double_tap = true,
inputbox = nil,
layout = {},

width = self.width*3,
height = nil,
bordersize = Size.border.default,
padding = Size.padding.small,
key_padding = Size.padding.default,
}

function popup_focus_manager:onTapClose(arg, ges)
if ges.pos:notIntersectWith(self.dimen) then
UIManager:close(self)
-- Allow ReaderLink to check if our dismiss tap
-- was itself on another footnote, and display
-- it. This avoids having to tap 2 times to
-- see another footnote.
if self.on_tap_close_callback then
self.on_tap_close_callback(arg, ges, self.height)
elseif self.close_callback then
self.close_callback(self.height)
end
return true
end
return false
end

function popup_focus_manager:onClose()
UIManager:close(self)
return true
end

function popup_focus_manager:onPressKey()
self:getFocusItem():handleEvent(Event:new("TapSelect"))
return true
end

local key_chars = self.key_chars
--logger.dbg(key_chars)
--error()
local extra_key_chars = {}

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

This here is the basic principle on which it's based: a 12-key grid of 4*3, manually assigned. 9 of those are directly accessible through a tap and 8 swipe directions.

It could be made to expand further on top, but that doesn't seem like the best idea as far as ease of use goes.

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 9, 2019

Contributor

Not sure while just reading the code: do all these keys are also tap'able (so, the whole thing works without swipe)? Or it's just showing what's available?
I guess so, if there are these extra keys.
Do the extra_key row shows when there is none defined for that key?

Also, if this is shown on Hold, you might want to handle HoldPan/HoldPanRelase to support a swipe started from a Hold :)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

These are only tappable. I suppose making swiping possible could be an idea.

extra_key_chars[1] = key_chars[2]
extra_key_chars[2] = key_chars[3]
extra_key_chars[3] = key_chars[4]
local top_key_chars = {}
top_key_chars[1] = key_chars.northwest
top_key_chars[2] = key_chars.north
top_key_chars[3] = key_chars.northeast
local middle_key_chars = {}
middle_key_chars[1] = key_chars.west
middle_key_chars[2] = key_chars[1]
middle_key_chars[3] = key_chars.east
local bottom_key_chars = {}
bottom_key_chars[1] = key_chars.southwest
bottom_key_chars[2] = key_chars.south
bottom_key_chars[3] = key_chars.southeast

local blank = VerticalSpan:new{width = self.width}

local vertical_group = VerticalGroup:new{}
local horizontal_group_extra = HorizontalGroup:new{}
local horizontal_group_top = HorizontalGroup:new{}
local horizontal_group_middle = HorizontalGroup:new{}
local horizontal_group_bottom = HorizontalGroup:new{}

local function horizontalRow(chars, group)
local layout_horizontal = {}
local i
for i = 1,3 do
local v = chars[i]
print(i)
print(v)
if v then
local virtual_key = VirtualKey:new{
key = v,
label = v,
keyboard = self.keyboard,
width = self.width,
height = self.height,
}
table.insert(group, virtual_key)
table.insert(layout_horizontal, virtual_key)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

Plus the padding.

else
table.insert(group, blank)
end
end
table.insert(popup_focus_manager.layout, layout_horizontal)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

It works well with the keyboard, go me! ;-) (Okay, just for thinking of it.)

end
horizontalRow(extra_key_chars, horizontal_group_extra)
horizontalRow(top_key_chars, horizontal_group_top)
horizontalRow(middle_key_chars, horizontal_group_middle)
horizontalRow(bottom_key_chars, horizontal_group_bottom)

table.insert(vertical_group, horizontal_group_extra)
table.insert(vertical_group, horizontal_group_top)
table.insert(vertical_group, horizontal_group_middle)
table.insert(vertical_group, horizontal_group_bottom)

local keyboard_frame = FrameContainer:new{
margin = 0,
bordersize = Size.border.default,
background = Blitbuffer.COLOR_WHITE,
radius = 0,
padding = self.keyboard.padding,
CenterContainer:new{

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

I haven't really thought about positioning yet. It needs to go on top of the key, but it shouldn't go outside the screen.

This comment has been minimized.

Copy link
@NiLuJe

NiLuJe Apr 9, 2019

Member

Yeah, this is going to be tricky with 'A', for instance, whether it's a qwerty or azerty layout ;).

I haven't checked what kind of info we have, but the simplest solution I can think of is to shift the whole popup to the opposite direction by the overflow (i.e., off-screen) amount, so that it's at the edge of the screen, while staying as close as possible around the original key, in these cases.

Sidebar: I'd make the border around the original key bolder/a different color to make it more obvious, which'd be neat in general, but pretty much necessary when it gets shifted like that ;).

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

However, it's only tricky with characters like the A if we want to use the full 12 of them. It's really easy to do it in the definition instead.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

I mean like this:

Screenshot_2019-04-09_17-20-26

local _a_ = {
    [1] = "a",
    north = "ë",
    northeast = "é",
    --northwest = "è",
    east = "ê",
    --west = "ẽ",
    south = "ę",
    southeast = "",
    --southwest = "ė",
    [3] = "ē",
}

This comment has been minimized.

Copy link
@NiLuJe

NiLuJe Apr 9, 2019

Member

True, but it breaks the swipe paradigm a tiny bit (and, for instance, we do kind of like our à's in french ;)), and has to be layout-specific.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

I think those swipes are already broken by their location regardless. But, um, everything's layout-specific, so I'm not quite sure what you mean there. ;-)

Light-gray highlight:

Screenshot_2019-04-09_17-50-47

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 9, 2019

Contributor

And ê should probably be associated to swipe north, as the ^ points up.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

The acute & grave accents happen to correspond to mnemonic gestures in this layout, but in the English/international keyboard I think the diaeresis makes more sense in the prime location than the more or less French-only circumflex. Besides, most diacritics are on top. ;-)

That being said, I haven't done an actual usage frequency analysis.

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 9, 2019

Contributor

OK, so that will be for the french layout only then.
(It's going to be a bit of a mess to keep the layouts sync'ed...)

dimen = Geom:new{
w = self.width*3 - 2*Size.border.default - 2*self.keyboard.padding,
h = self.height*4 - 2*Size.border.default - 2*self.keyboard.padding,
},
vertical_group,
}
}
--self[1] = BottomContainer:new{
-- dimen = Screen:getSize(),
-- keyboard_frame,
--}
--self.dimen = keyboard_frame:getSize()

popup_focus_manager[1]=keyboard_frame

popup_focus_manager.ges_events = {
TapClose = {
GestureRange:new{
ges = "tap",
range = range,
}
},
}

if Device:hasDPad() then
popup_focus_manager.key_events.PressKey = { {"Press"}, doc = "select key" }
end
if Device:hasKeys() then
popup_focus_manager.key_events.Close = { {"Back"}, doc = "close keyboard" }
end

--self:_refresh()
--UIManager:show(keyboard_frame)
UIManager:show(popup_focus_manager)
--error()

UIManager:setDirty(self, function()

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 9, 2019

Author Member

This stuff down here is super messy obviously, just for quick testing.

return "ui", keyboard_frame:getSize()
end)

end
self.swipe_callback = function(ges)
self.keyboard:addChar(self.key_chars[ges.direction])
end
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.