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

Kindle4NT improvements #3745

Merged
merged 22 commits into from Mar 14, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion frontend/apps/filemanager/filemanager.lua
Expand Up @@ -362,7 +362,8 @@ function FileManager:init()
end

if Device:hasKeys() then
self.key_events.Close = { {"Home"}, doc = "Close file manager" }
self.key_events.Home = { {"Home"}, doc = "go home" }
self.file_chooser.key_events.Close = nil
Copy link
Contributor

@poire-z poire-z Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm so used to Backspace to quit things on the emulator, which work(ed) on quite everything. You removed them here and elsewhere.
No real way to keep that while staying ok with what you want?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, I had the feeling this would not be ok.
At first, I tried to add a command parameter to reader.lua, something like "reader.lua --no-key-escape", but there were no example, so I had no idea were the variable sould be stored.
I will try to think of something.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure @poire-z can also get used to Alt+F4 instead. :-P I think backspace or escape makes sense to quit dialogs, but I'm not so sure about the program itself.

But I am somewhat confused by some of these changes, like what is Prec? Could you describe what's happening right now and how these keybindings are better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sorry, "Prec" is supposed to be "Back", it should go to upper menu or close the touchmenu.
I tried to look to make the keybindings Escape to exit but the escape key doesn't seem to be defined in event_map_sdl2.lua

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add it, of course. ;-)

[41] = "Back", -- Escape

(Or something like that.)

end

self:handleEvent(Event:new("SetDimensions", self.dimen))
Expand Down Expand Up @@ -788,4 +789,10 @@ function FileManager:moveFile(from, to)
return util.execute(self.mv_bin, from, to) == 0
end

function FileManager:onHome()
print('go home')
return self:goHome()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray newline

end

return FileManager
2 changes: 1 addition & 1 deletion frontend/apps/filemanager/filemanagermenu.lua
Expand Up @@ -306,7 +306,7 @@ function FileManagerMenu:onShowMenu()
}

local main_menu
if Device:isTouchDevice() then
if Device:hasKeys() then
local TouchMenu = require("ui/widget/touchmenu")
main_menu = TouchMenu:new{
width = Screen:getWidth(),
Expand Down
3 changes: 1 addition & 2 deletions frontend/apps/reader/modules/readermenu.lua
Expand Up @@ -53,7 +53,6 @@ function ReaderMenu:init()
self.registered_widgets = {}

if Device:hasKeys() then
self.key_events = { Close = { { "Back" }, doc = "close menu" }, }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will you close the menu?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The back key is directly handled by touchemenu.

if Device:isTouchDevice() then
self.key_events.TapShowMenu = { { "Menu" }, doc = "show menu", }
else
Expand Down Expand Up @@ -241,7 +240,7 @@ function ReaderMenu:onShowReaderMenu(tab_index)
}

local main_menu
if Device:isTouchDevice() then
if Device:hasKeys() then
Copy link
Contributor

@poire-z poire-z Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Here and elsewhere)
There are devices which are TouchDevices and don't have keys, actually most :)
With this (I guess), when I touch top, i don't get my touch menu, but a new window "Document menu" I have never ever seen :) .
May be if Device:isTouchDevice() or Device:hasKeys() then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes of course, fixing it asap

local TouchMenu = require("ui/widget/touchmenu")
main_menu = TouchMenu:new{
width = Screen:getWidth(),
Expand Down
15 changes: 7 additions & 8 deletions frontend/apps/reader/readerui.lua
Expand Up @@ -54,8 +54,8 @@ local ReaderUI = InputContainer:new{
name = "ReaderUI",

key_events = {
Close = { { "Home" },
doc = "close document", event = "Close" },
Home = { { "Home" },
doc = "open file browser", event = "Home" },
},
active_widgets = {},

Expand Down Expand Up @@ -97,12 +97,6 @@ function ReaderUI:init()
self.dialog = self
end

if Device:hasKeys() then
self.key_events.Back = {
{ "Back" }, doc = "close document",
event = "Close" }
end

self.doc_settings = DocSettings:open(self.document.file)

-- a view container (so it must be child #1!)
Expand Down Expand Up @@ -622,4 +616,9 @@ function ReaderUI:dealWithLoadDocumentFailure()
error("crengine failed recognizing or parsing this file: unsupported or invalid document")
end

function ReaderUI:onHome()
print("go home2")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could potentially use logger.dbg("some relevant message") btw.

return self:showFileManager()
end

return ReaderUI
1 change: 1 addition & 0 deletions frontend/device/kindle/device.lua
Expand Up @@ -317,6 +317,7 @@ function Kindle4:init()
}
self.input.open("/dev/input/event0")
self.input.open("/dev/input/event1")
self.input.open("fake_events")
Kindle.init(self)
end

Expand Down
10 changes: 10 additions & 0 deletions frontend/ui/widget/bookstatuswidget.lua
Expand Up @@ -27,6 +27,8 @@ local util = require("util")
local _ = require("gettext")
local Screen = require("device").screen
local template = require("ffi/util").template
local Device = require("device")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding Device here requires putting it alphbetically in order (on line 5).

That in turn means local Screen = require("device").screen should be changed to local Screen = Device.screen


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray newline


local stats_book = {}

Expand Down Expand Up @@ -82,6 +84,14 @@ function BookStatusWidget:init()
show_parent = self,
readonly = self.readonly,
}

if Device:hasKeys() then
self.key_events = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Four more spaces in this line and the next two please.

AnyKeyPressed = { { Device.input.group.Any },
seqtext = "any key", doc = "close dialog" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment that this is (presumably) so you won't get locked in?

}
end

local screen_size = Screen:getSize()
self[1] = FrameContainer:new{
width = screen_size.w,
Expand Down
52 changes: 24 additions & 28 deletions frontend/ui/widget/focusmanager.lua
Expand Up @@ -57,56 +57,52 @@ function FocusManager:onFocusMove(args)
end
local current_item = self.layout[self.selected.y][self.selected.x]
while true do
if self.selected.x + dx > #self.layout[self.selected.y]
or self.selected.x + dx < 1 then
break -- abort when we run into horizontal borders
end

-- call widget wrap callbacks in vertical direction
if self.selected.y + dy > #self.layout then
if not self:onWrapLast() then
break
end
elseif self.selected.y + dy < 1 then
if not self:onWrapFirst() then
if not self.layout[self.selected.y + dy] then
--vertical borders, try to wraparound
if not self:warpAround(dy) then
break
end
else
self.selected.y = self.selected.y + dy
if #self.layout[self.selected.y] == 0 then -- horizontal separator
self.selected.y = self.selected.y + dy -- skip it
end
end
self.selected.x = self.selected.x + dx
if self.selected.x > #self.layout[self.selected.y] then
-- smaller nb of items on new row than on prev row
self.selected.x = #self.layout[self.selected.y]

if not self.layout[self.selected.y][self.selected.x + dx] then
--vertical border, no wraparound
break
else
self.selected.x = self.selected.x + dx
end

if self.layout[self.selected.y][self.selected.x] ~= current_item
or not self.layout[self.selected.y][self.selected.x].is_inactive then
-- we found a different object to focus
current_item:handleEvent(Event:new("Unfocus"))
self.layout[self.selected.y][self.selected.x]:handleEvent(Event:new("Focus"))
-- trigger a repaint (we need to be the registered widget!)
-- trigger a fast repaint, this seem to not count toward a fullscreen eink resfresh
-- TODO: is this really needed?
UIManager:setDirty(self.show_parent or self, "partial")
UIManager:setDirty(self.show_parent or self, "fast")
break
end
end

return true
end

function FocusManager:onWrapFirst()
self.selected.y = #self.layout
return true
function FocusManager:warpAround(dy)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warpAround or wrapAround ? :)

--go to the last valid item directly above or below the current item
--return false if none could be found
local y = self.selected.y
while self.layout[y - dy] and self.layout[y - dy][self.selected.x] do
y = y - dy
end
if y ~= self.selected.y then
self.selected.y = y
return true
else
return false
end
end

function FocusManager:onWrapLast()
self.selected.y = 1
return true
end

function FocusManager:getFocusItem()
return self.layout[self.selected.y][self.selected.x]
Expand Down
17 changes: 17 additions & 0 deletions frontend/ui/widget/iconbutton.lua
Expand Up @@ -124,4 +124,21 @@ function IconButton:onHoldIconButton()
return true
end


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I'm just nitpicking nothings a remaining stray newline.

function IconButton:onFocus()
--quick and dirty, need better way to show focus
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but messy here :-P

self.image.invert=true
return true
end

function IconButton:onUnfocus()
self.image.invert=false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 spaces?

return true
end

function IconButton:onTapSelect()
self:onTapIconButton()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

end


return IconButton
6 changes: 6 additions & 0 deletions frontend/ui/widget/inputtext.lua
Expand Up @@ -88,6 +88,12 @@ if Device.isTouchDevice() then
end)
end
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray newlines


elseif not Device.hasKeyboard() then
Keyboard = require("ui/widget/virtualkeyboard")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation

function InputText:initEventListener() end --do nothing but doesnt crash for now

else
Keyboard = require("ui/widget/physicalkeyboard")
function InputText:initEventListener() end
Expand Down
55 changes: 52 additions & 3 deletions frontend/ui/widget/touchmenu.lua
Expand Up @@ -26,6 +26,10 @@ local util = require("ffi/util")
local _ = require("gettext")
local Screen = Device.screen
local getMenuText = require("util").getMenuText
local FocusManager = require("ui/widget/focusmanager")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/koreader/koreader-base/wiki/Coding-style#order-of-requires

FocusManager, Event and UnderlineContainer should go up above.

Input can stay here, one line above Screen.

local Event = require("ui/event")
local Input = Device.input
local UnderlineContainer = require("ui/widget/container/underlinecontainer")

--[[
TouchMenuItem widget
Expand Down Expand Up @@ -97,7 +101,32 @@ function TouchMenuItem:init()
},
},
}
self[1] = self.item_frame


self._underline_container = UnderlineContainer:new{
vertical_align = "center",
dimen =self.dimen,
self.item_frame
}


self[1] = self._underline_container

function self:isEnabled()
return item_enabled ~= false and true
end

end


function TouchMenuItem:onFocus()
self._underline_container.color = Blitbuffer.COLOR_BLACK
return true
end

function TouchMenuItem:onUnfocus()
self._underline_container.color = Blitbuffer.COLOR_WHITE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation (& some stray newlines up above)

return true
end

function TouchMenuItem:onTapSelect(arg, ges)
Expand Down Expand Up @@ -202,9 +231,11 @@ function TouchMenuBar:init()
callback = nil,
padding_left = icon_padding,
padding_right = icon_padding,
menu = self.menu,
}

table.insert(self.icon_widgets, ib)
table.insert(self.menu.layout, ib) -- for the focusmanager

-- we have to use local variable here for closure callback
local _start_seg = end_seg + icon_sep_width
Expand Down Expand Up @@ -319,7 +350,7 @@ end
--[[
TouchMenu widget for hierarchical menus
--]]
local TouchMenu = InputContainer:new{
local TouchMenu = FocusManager:new{
tab_item_table = {},
-- for returnning in multi-level menus
item_table_stack = nil,
Expand Down Expand Up @@ -351,6 +382,8 @@ function TouchMenu:init()
end
end

self.layout = {}

self.ges_events.TapCloseAllMenus = {
GestureRange:new{
ges = "tap",
Expand All @@ -368,7 +401,10 @@ function TouchMenu:init()
}
}

self.key_events.Close = { {"Back"}, doc = "close touch menu" }
self.key_events.Prec = { {"Back"}, doc = "back" }
self.key_events.NextPage = { {Input.group.PgFwd}, doc = "next page" }
self.key_events.PrevPage = { {Input.group.PgBack}, doc = "previous page" }
self.key_events.Press = { {"Press"}, doc = "chose selected item" }

local icons = {}
for _,v in ipairs(self.tab_item_table) do
Expand Down Expand Up @@ -509,7 +545,9 @@ function TouchMenu:updateItems()
local old_dimen = self.dimen and self.dimen:copy()
self:_recalculatePageLayout()
self.item_group:clear()
self.layout = {} --aurel
table.insert(self.item_group, self.bar)
table.insert(self.layout, self.bar.icon_widgets) --for the focusmanager

for c = 1, self.perpage do
-- calculate index in item_table
Expand All @@ -526,6 +564,9 @@ function TouchMenu:updateItems()
show_parent = self.show_parent,
}
table.insert(self.item_group, item_tmp)
if item_tmp:isEnabled() then
table.insert(self.layout, {[self.cur_tab] = item_tmp}) --for the focusmanager
end
if item.separator and c ~= self.perpage then
-- insert split line
table.insert(self.item_group, self.split_line)
Expand Down Expand Up @@ -553,6 +594,7 @@ function TouchMenu:updateItems()
-- recalculate dimen based on new layout
self.dimen.w = self.width
self.dimen.h = self.item_group:getSize().h + self.bordersize*2 + self.padding*2
self.selected = { x = self.cur_tab, y = 1 } --reset the position of the focusmanager

UIManager:setDirty("all", function()
local refresh_dimen =
Expand Down Expand Up @@ -706,5 +748,12 @@ end
function TouchMenu:onClose()
self:closeMenu()
end
function TouchMenu:onPrec()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lack of newline before

self:backToUpperMenu()
end
function TouchMenu:onPress()
self:getFocusItem():handleEvent(Event:new("TapSelect"))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray newline

end

return TouchMenu