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] Gesture manager: corner gestures #4878

Merged
merged 20 commits into from Apr 7, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -37,6 +37,10 @@ read_globals = {
"DTAP_ZONE_BACKWARD",
"DTAP_ZONE_BOOKMARK",
"DTAP_ZONE_FLIPPING",
"DTAP_ZONE_TOP_LEFT",
"DTAP_ZONE_TOP_RIGHT",
"DTAP_ZONE_BOTTOM_LEFT",
"DTAP_ZONE_BOTTOM_RIGHT",
"DDOUBLE_TAP_ZONE_NEXT_CHAPTER",
"DDOUBLE_TAP_ZONE_PREV_CHAPTER",
"DCHANGE_WEST_SWIPE_TO_EAST",
@@ -64,17 +64,21 @@ DOVERLAPPIXELS = 30
FOLLOW_LINK_TIMEOUT = 0.5

-- customizable tap zones(rectangles)
-- x: x coordinate of top left corner in proportion of screen width
-- y: y coordinate of top left corner in proportion of screen height
-- w: width of tap zone in proportion of screen width
-- h: height of tap zone in proportion of screen height
-- x: x coordinate of top left corner in proportion to screen width
-- y: y coordinate of top left corner in proportion to screen height
-- w: tap zone width in proportion to screen width
-- h: tap zone height in proportion to screen height
DTAP_ZONE_MENU = {x = 1/8, y = 0, w = 3/4, h = 1/8}
DTAP_ZONE_CONFIG = {x = 1/8, y = 7/8, w = 3/4, h = 1/8}
DTAP_ZONE_MINIBAR = {x = 0, y = 31/32, w = 1, h = 1/32}
DTAP_ZONE_FORWARD = {x = 1/4, y = 0, w = 3/4, h = 1}
DTAP_ZONE_BACKWARD = {x = 0, y = 0, w = 1/4, h = 1}
DTAP_ZONE_BOOKMARK = {x = 7/8, y = 0, w = 1/8, h = 1/8}
DTAP_ZONE_FLIPPING = {x = 0, y = 0, w = 1/8, h = 1/8}
-- DTAP_ZONE_BOOKMARK = {x = 7/8, y = 0, w = 1/8, h = 1/8} -- deprecated
-- DTAP_ZONE_FLIPPING = {x = 0, y = 0, w = 1/8, h = 1/8} -- deprecated
DTAP_ZONE_TOP_LEFT = {x = 0, y = 0, w = 1/8, h = 1/8}
DTAP_ZONE_TOP_RIGHT = {x = 7/8, y = 0, w = 1/8, h = 1/8}
DTAP_ZONE_BOTTOM_LEFT = {x = 0, y = 7/8, w = 1/8, h = 1/8}
DTAP_ZONE_BOTTOM_RIGHT = {x = 7/8, y = 7/8, w = 1/8, h = 1/8}
DDOUBLE_TAP_ZONE_NEXT_CHAPTER = {x = 6/8, y = 0, w = 2/8, h = 2/8}
DDOUBLE_TAP_ZONE_PREV_CHAPTER = {x = 0, y = 0, w = 2/8, h = 2/8}

@@ -88,7 +88,7 @@ function FileManager:init()
padding_left = Size.padding.large,
padding_right = Size.padding.large,
padding_bottom = 0,
callback = function() self:tapPlus() end,
callback = nil, -- top right corner callback handled by gesture manager

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 6, 2019

Contributor

Might be a first (drawing a button, no callback, but having it somethow triggered by some remote overlay, hoping the regions of each will keep overlaping as we make changes :)
But I don't see how else easily...

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 6, 2019

Author Member

I could take it back out again. Maybe I'll think of something after a night's sleep. ;-)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 7, 2019

Author Member

Btw, not really a first. This is basically exactly what the flipping & bookmark corners do.

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 7, 2019

Contributor

They do not feel like that to me. ReaderDogear and ReaderFlipping widgets both are painted (and so are containers of their own inner widgets), set the tap zone, and handle the action.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 7, 2019

Author Member

They might technically set the tap zone, but the tap zone they set is completely unrelated to the actual widget.

if Device:isTouchDevice() then
self.ges_events = {
Tap = {
GestureRange:new{
ges = "tap",
range = Geom:new{
x = new_screen_width*DTAP_ZONE_FLIPPING.x,
y = new_screen_height*DTAP_ZONE_FLIPPING.y,
w = new_screen_width*DTAP_ZONE_FLIPPING.w,
h = new_screen_height*DTAP_ZONE_FLIPPING.h
}
}
}
}
end

if Device:isTouchDevice() then
self.ges_events = {
Tap = {
GestureRange:new{
ges = "tap",
range = Geom:new{
x = new_screen_width*DTAP_ZONE_BOOKMARK.x,
y = new_screen_height*DTAP_ZONE_BOOKMARK.y,
w = new_screen_width*DTAP_ZONE_BOOKMARK.w,
h = new_screen_height*DTAP_ZONE_BOOKMARK.h
}
}
}
}
end

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 7, 2019

Contributor

I was not referring about tap zone overlaping with the icon. Just the self containment of the 3 parts involved (widget, tap zone, action) in a single InputContainer.
What bothers me is the "remote overlay", the tap zone being set far away in readergesture.
But now, it's no more a first, but a second and a third :)
But again, I don't see how else.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 7, 2019

Author Member

True, it's nice that they're self-contained units.

I think the "proper" way would be to allow the user to set their own icon+action+possibly tap zone in place (including one that's transparent) so that they are more together.

In Opera's toolbar.ini that'd look like this:

[Document Toolbar.content]
NamedButton0, tbb_Back, -847360260="Back + Show hidden popup menu, "Back Menu""
NamedButton1, tbb_Forward, 830055518="Forward + Show hidden popup menu, "Internal Forward History""
Button2, -108388079="Fast Forward, , , "Next" + Show hidden popup menu, "Internal Fast Forward History""
Button3, -119414254="Wand | Fast Forward, , , "Next" + Show hidden popup menu, "Internal Fast Forward History""
Address4
Button5, "''InO"="Execute program, "opera-developer", "%u", "InO", "OperaBlink""
Button6, "InFx"="Execute program, "firefox", "%u", "InFx", "Firefox""
Zoom7
ExtensionSet8

In Otter it's done with JSON:

[
	{
		"actions": [
			"RewindAction",
			"GoBackAction",
			"GoForwardAction",
			"FastForwardAction",
			"ReloadOrStopAction",
			"AddressWidget",
			"SearchWidget"
		],
		"buttonStyle": "iconOnly",
		"fullScreenVisibility": "hidden",
		"identifier": "AddressBar",
		"normalVisibility": "visible",
		"row": 0,
		"title": "Address Bar"
	},
	{
		"buttonStyle": "iconOnly",
		"currentPanel": "bookmarks",
		"fullScreenVisibility": "hidden",
		"identifier": "SideBar",
		"location": "left",
		"normalVisibility": "visible",
		"panelSize": 530,
		"panels": [
			"bookmarks",
			"cookies",
			"history",
			"notes",
			"passwords",
			"transfers",
			"windows",
			"config"
		],
		"row": 0,
		"title": "Sidebar"
	}
]

That way you've got clear separation of concern in the code but everything is also clearly defined together and configurable. We've currently got a bit of an awkward in-between.

Also I don't have a clear vision yet for the outcome, but I do know that with all the actions that've been added to the gesture manager it's much easier to move to a different/further abstraction than it was a few months ago.

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 7, 2019

Contributor

Well, I hope we don't need all that :)
Also, we're in a bit different situation as the code is in Lua, so we don't really need to provide as much hooks for customisation as the apps in compiled C that are not hackable - the interested users (very little) can always hack/patch/monkey-patch our code base to have things a bit more to their liking (I do that). Complexifying and abstracting with different levels of indirection to satisfy a small user base is not really to my liking :)

But yes, with gesturemanager, it has become different - you could possibly move everything reader/filemanager gestures in there to have them in a single place. But that would be one global level of indirection that would make reading/hacking the code a bit more tedious.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 7, 2019

Author Member

Architecturally speaking it may be better for the gesture manager to be more like the plugin manager. Actually it's already quite a bit like that with the event system. But anyway, it's working and I have no special desire to waste time redesigning it or whatnot. ;-)

}

self.path_text = TextWidget:new{
@@ -404,6 +404,11 @@ function FileChooser:onBack()
end
end

function FileManager:onShowPlusMenu()
self:tapPlus()
return true
end

function FileManager:tapPlus()
local buttons = {
{
@@ -1,11 +1,9 @@
local Device = require("device")
local Event = require("ui/event")
local Geom = require("ui/geometry")
local GestureRange = require("ui/gesturerange")
local ImageWidget = require("ui/widget/imagewidget")
local InputContainer = require("ui/widget/container/inputcontainer")
local RightContainer = require("ui/widget/container/rightcontainer")
local Screen = require("device").screen
local Screen = Device.screen

local ReaderDogear = InputContainer:new{}

@@ -70,30 +68,9 @@ end
function ReaderDogear:resetLayout()
local new_screen_width = Screen:getWidth()
if new_screen_width == self._last_screen_width then return end
local new_screen_height = Screen:getHeight()
self._last_screen_width = new_screen_width

self[1].dimen.w = new_screen_width
if Device:isTouchDevice() then
self.ges_events = {
Tap = {
GestureRange:new{
ges = "tap",
range = Geom:new{
x = new_screen_width*DTAP_ZONE_BOOKMARK.x,
y = new_screen_height*DTAP_ZONE_BOOKMARK.y,
w = new_screen_width*DTAP_ZONE_BOOKMARK.w,
h = new_screen_height*DTAP_ZONE_BOOKMARK.h
}
}
}
}
end
end

function ReaderDogear:onTap()
self.ui:handleEvent(Event:new("ToggleBookmark"))
return true
end

function ReaderDogear:onSetDogearVisibility(visible)
@@ -1,11 +1,8 @@
local Geom = require("ui/geometry")
local ImageWidget = require("ui/widget/imagewidget")
local InputContainer = require("ui/widget/container/inputcontainer")
local LeftContainer = require("ui/widget/container/leftcontainer")
local ImageWidget = require("ui/widget/imagewidget")
local GestureRange = require("ui/gesturerange")
local Device = require("device")
local Geom = require("ui/geometry")
local Screen = require("device").screen
local Event = require("ui/event")

local ReaderFlipping = InputContainer:new{
orig_reflow_mode = 0,
@@ -25,36 +22,9 @@ end
function ReaderFlipping:resetLayout()
local new_screen_width = Screen:getWidth()
if new_screen_width == self._last_screen_width then return end
local new_screen_height = Screen:getHeight()
self._last_screen_width = new_screen_width

self[1].dimen.w = new_screen_width
if Device:isTouchDevice() then
self.ges_events = {
Tap = {
GestureRange:new{
ges = "tap",
range = Geom:new{
x = new_screen_width*DTAP_ZONE_FLIPPING.x,
y = new_screen_height*DTAP_ZONE_FLIPPING.y,
w = new_screen_width*DTAP_ZONE_FLIPPING.w,
h = new_screen_height*DTAP_ZONE_FLIPPING.h
}
}
}
}
end
end

function ReaderFlipping:onTap()
if not self.ui.document.info.has_pages then
-- ReaderRolling has no support (yet) for onTogglePageFlipping,
-- so don't make that top left tap area unusable (and allow
-- taping on links there)
return false
end
self.ui:handleEvent(Event:new("TogglePageFlipping"))
return true
end

return ReaderFlipping
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.