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

Conversation

Projects
None yet
2 participants
@Frenzie
Copy link
Member

commented Apr 6, 2019

Cf. #4877.

@Frenzie Frenzie added the UX label Apr 6, 2019

@Frenzie Frenzie added this to the 2019.05 milestone Apr 6, 2019

Frenzie added some commits Apr 6, 2019

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2019

I wonder if we should add more overrides. cf #4856 (comment).
Some values may be tweaked by eg. DTAP_ZONE_MENU, so they could overlap with these tap corners.
As these are toggable, I think they should override any of the other ones, whether by default not overlapping, or overlapping if changed by the user (to avoid that random ordering, different by session).

@Frenzie

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

Good point, I added backward for the same reason.

@Frenzie

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

Hold corner

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2019

:)

Not readerconfigmenu_tap among the overrides?
Also, it has never been an override, but I thought readerhighlight_tap had a top priority. But just checking on my Kobo, I can bookmark when taping on some text highlighted at top of page.
So, dogear somehow overrides (might be random) - or it was using the 2nd gesture infrastructure that I may be didn't check... but I thought it had lower priority. A bit confused.
Same for tap on links, that wasn't on my list - so the other gesture infrasctructure that don't give names and are not overrid'able.

(Hold corners in a submenu, but not the tap corners?)

Frenzie added some commits Apr 6, 2019

@Frenzie

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

(Hold corners in a submenu, but not the tap corners?)

Yeah, I knew that was coming. :-P

I'm not entirely sure if they should be because they're the most accessible gestures.

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2019

I'm not entirely sure if they should be because they're the most accessible gestures.

Yeah, it's fine as is (it leaves an option for when this menu exceeds 1 page).

Frenzie added some commits Apr 6, 2019

@Frenzie Frenzie changed the title [UX] Migrate top-right corner to gesture manager [UX] Gesture manager: top right corner and more corner gestures Apr 6, 2019

DTAP_ZONE_BOOKMARK = {x = 7/8, y = 0, w = 1/8, h = 1/8}
-- top left corner
DTAP_ZONE_FLIPPING = {x = 0, y = 0, w = 1/8, h = 1/8}

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 6, 2019

Contributor

Can't you migrate these two to DTAP_ZONE_BOTTOM_LEFT like the other ones? And use where they are used

if DTAP_ZONE_BOOKMARK  then -- legacy global variable, may still be defined in default.persistent.lua
  -- use DTAP_ZONE_BOOKMARK
else
  -- use DTAP_ZONE_TOP_RIGHT
end

or luacheck would complain ?

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 6, 2019

Author Member

No, should be fine.

@@ -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. ;-)

@Frenzie Frenzie changed the title [UX] Gesture manager: top right corner and more corner gestures [UX] Gesture manager: corner gestures Apr 7, 2019

@Frenzie Frenzie merged commit 8466af2 into koreader:master Apr 7, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@Frenzie Frenzie deleted the Frenzie:top-right-corner branch Apr 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.