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 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

migrate top-right fm corner

  • Loading branch information...
Frenzie committed Apr 6, 2019
commit a6f91b1e78ce2ae65221c9b606b9a0c82e093dfa
@@ -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,10 @@ function FileChooser:onBack()
end
end

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

function FileManager:tapPlus()
local buttons = {
{
@@ -79,6 +79,7 @@ local action_strings = {
zoom_page = _("Zoom to fit page"),

folder_up = _("Folder up"),
show_plus_menu = _("Show plus menu"),
folder_shortcuts = _("Folder shortcuts"),
cycle_highlight_action = _("Cycle highlight action"),
wallabag_download = _("Wallabag retrieval"),
@@ -145,7 +146,7 @@ function ReaderGesture:init()
self.is_docless = self.ui == nil or self.ui.document == nil
self.ges_mode = self.is_docless and "gesture_fm" or "gesture_reader"
self.default_gesture = {
tap_top_right_corner = self.ges_mode == "gesture_reader" and "toggle_bookmark" or "nothing",
tap_top_right_corner = self.ges_mode == "gesture_reader" and "toggle_bookmark" or "show_plus_menu",
tap_right_bottom_corner = "nothing",
tap_left_bottom_corner = Device:hasFrontlight() and "toggle_frontlight" or "nothing",
short_diagonal_swipe = "full_refresh",
@@ -295,7 +296,6 @@ function ReaderGesture:addToMainMenu(menu_items)
self:genMultiswipeSubmenu(),
{
text = _("Tap top right corner"),
enabled_func = function() return self.ges_mode == "gesture_reader" end,
sub_item_table = self:buildMenu("tap_top_right_corner", self.default_gesture["tap_top_right_corner"]),
},
{
@@ -373,6 +373,7 @@ function ReaderGesture:buildMenu(ges, default)
{"clear_location_history", not self.is_docless, true},

{"folder_up", self.is_docless},
{"show_plus_menu", self.is_docless},
{"folder_shortcuts", true, true},

{ "toc", not self.is_docless},
@@ -801,6 +802,8 @@ function ReaderGesture:gestureAction(action, ges)
end
elseif action == "folder_up" then
self.ui.file_chooser:changeToPath(string.format("%s/..", self.ui.file_chooser.path))
elseif action == "show_plus_menu" then
self.ui:handleEvent(Event:new("ShowPlusMenu"))
elseif action == "folder_shortcuts" then
self.ui:handleEvent(Event:new("ShowFolderShortcutsDialog"))
elseif action == "open_previous_document" then
@@ -92,6 +92,7 @@ function IconButton:initGesListener()
end

function IconButton:onTapIconButton()
if not self.callback then return end
if G_reader_settings:isFalse("flash_ui") then
self.callback()
else
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.