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

Wish: Enter Go to page from Skim mode dialog #3526

Closed
ersi-dnd opened this issue Dec 1, 2017 · 19 comments
Closed

Wish: Enter Go to page from Skim mode dialog #3526

ersi-dnd opened this issue Dec 1, 2017 · 19 comments
Labels

Comments

@ersi-dnd
Copy link

ersi-dnd commented Dec 1, 2017

Thank you for this latest release (koreader-nightly-20171023). I haven't installed it yet on all my devices, but on the one I have it looks awesome.

There is this minor issue: Skim mode dialog reads "Go to page" as its title. I think it should say "Skim mode" instead.

And I have a wish: In the Go to page dialog, we have the button "Skim mode" which enters Skim mode. How about adding a Go to page button in the Skim mode dialog?

@ersi-dnd ersi-dnd changed the title Wish: Make Go to page and Skim mode interchangeable Wish: Enter Go to page from Skim mode dialog Dec 1, 2017
@robert00s
Copy link
Contributor

And I have a wish: In the Go to page dialog, we have the button "Skim mode" which enters Skim mode. How about adding a Go to page button in the Skim mode dialog?

Try to tap in page number to go back from "Skim mode" to "Go to page"

@ersi-dnd
Copy link
Author

ersi-dnd commented Dec 1, 2017

@robert00s
Wow, it works! But shouldn't the page number in the Skim dialog look like a button, just like everything else around it? This way it would occur to people to press it.

@ersi-dnd
Copy link
Author

ersi-dnd commented Dec 2, 2017

Looking at the Go to page and Skim mode dialogs, looks like Skim mode dialog is meant to be an extension to the Go to page dialog: In Go to page dialog, press Skim mode, and you can do as if more stuff, that's why the "Go to page" title stays there.

I have this suggestion: Make Skim mode an actual expansion of the Go to page dialog, i.e. retain the full Go to page dialog when Skim mode opens. When the user presses Skim mode, then instead of switching away from Go to page dialog, make Go to page dialog expand to include the Skim mode controls.

This leaves us with the keyboard detail: When Skim mode dialog opens, the keyboard vanishes (probably because the Skim mode dialog is so big). My idea is this: The new expanded dialog includes the Go to page form. When the user places the focus there, the Skim mode controls should vanish and the keyboard reappear.

(All this is unnecessary, really, only cosmetics. As it is, all the functions are there, I just found it confusing that the Skim mode is titled Go to page.)

@Frenzie Frenzie added the UX label Mar 6, 2018
@robert00s
Copy link
Contributor

I have this suggestion: Make Skim mode an actual expansion of the Go to page dialog, i.e. retain the full Go to page dialog when Skim mode opens. When the user presses Skim mode, then instead of switching away from Go to page dialog, make Go to page dialog expand to include the Skim mode controls.

Let's leave as it is :)
After your suggestion Go to page/Skim mode widget will we too big.

I just found it confusing that the Skim mode is titled Go to page.)

I'll prepare PR for this soon.

Note that skim mode is the default mode when long press on status bar for serveral weeks.

@ersi-dnd
Copy link
Author

ersi-dnd commented Sep 14, 2018

@robert00s

After your suggestion Go to page/Skim mode widget will we too big.

Yes, could be. Another thing that most definitely is too big is Table of Contents. Mostly that thing is entirely empty, and to get out of it there is a too tiny cross in the corner. I'd like the Table of Contents dialog to be small (particularly when it's empty, or, even better, not show up at all when there is nothing to show; or have a big fat Cancel button on it instead of the little cross in the corner) so I can hit anywhere around it to get rid of it, just like with Go to page dialog. And/Or there could be a shortcut button to Bookmarks somewhere in the Table of Contents dialog, just like you can get back and forth between Go to and Skim to dialogs.

I see that you merged something related to this issue. I updated Koreader now, but the skim dialog still reads Go to page on top. Did I update too fast?

@poire-z
Copy link
Contributor

poire-z commented Sep 15, 2018

Did I update too fast?

Yes, not yet merged, still discussed in #4223.

Another thing that most definitely is too big is Table of Contents

(Well, if you see that many empty ToC, why are you hitting this menu that often? :) already requested in #3997)
It shouldn't be smaller when there is ToC content. I don't mind having it empty when no ToC, but I may not mind either if there was an InfoMessage instead. But it should show something, just for feedback, so you don't go thinking your gesture didn't work and try it again.

I discovered a few weeks ago that, for people with multitouch devices that use the 2-finger swipe to show ToC or Bookmarks, that you can close it with that same 2-finger swipe in the other direction.

But I ended it up adding for myself Swipe south to close these (because I was too often missing that little cross in the corner - but I don't think we should add a big fat button to this view, that I'd rather like clean of distractions).
Small patch, that makes it work with Toc, Bookmarks and History (but not with FileBrowser thanks to the conditions - so no conflict with Swipe to show menu):

--- orig_koreader/frontend/ui/widget/menu.lua
+++ koreader/frontend/ui/widget/menu.lua
 function Menu:onSwipe(arg, ges_ev)
     if ges_ev.direction == "west" then
         self:onNextPage()
     elseif ges_ev.direction == "east" then
         self:onPrevPage()
+    -- easier closing (than tap on button at top right)
+    elseif ges_ev.direction == "south" and self.has_close_button and not self.no_title then
+        self:onClose()
     else
         -- trigger full refresh
         UIManager:setDirty(nil, "full")
--- orig_koreader/plugins/coverbrowser.koplugin/covermenu.lua
+++ koreader/plugins/coverbrowser.koplugin/covermenu.lua
 function CoverMenu:onSwipe(arg, ges_ev)
     if ges_ev.direction == "west" then
         self:onNextPage()
     elseif ges_ev.direction == "east" then
         self:onPrevPage()
+    -- easier closing (than tap on button at top right)
+    elseif ges_ev.direction == "south" and self.has_close_button and not self.no_title then
+        self:onClose()
     elseif ges_ev.direction ~= "north" and ges_ev.direction ~= "south" then
         -- but not if north/south, and we're triggering menu
         -- trigger full refresh

Should I PR that?

@ersi-dnd
Copy link
Author

@poire-z

(Well, if you see that many empty ToC, why are you hitting this menu that often? :)

By mistake. The usual scenario is, when a book is open, I bring up the menus and the first menu with Table of contents on top is auto-expanded, I want to go to the top-right corner to the ☰ menu but it hits the Table of contents instead. So the next question in that situation is how to get rid of the Table of contents, but it's not easy to get rid of it.

Should I PR that?

What does this mean? I don't know what PR means.

I am slowly beginning to understand that you have some configuration options available for swipes. I would like to set them for the things I use most, such as switching nightmode on and off, getting in and out of bookmarks and history, etc. Are there instructions for this somewhere?

@poire-z
Copy link
Contributor

poire-z commented Sep 15, 2018

I don't know what PR means.

Pull Request. I was asking the other developers if I should push this change (that I made for myself) to the main code, so that everybody gets Swipe south to close TOC, Bookmarks, History.

I am slowly beginning to understand that you have some configuration options available for swipes

No, we don't have that. There's no way to change behaviour of gestures, except what is available thru menu that allows disabling them (opening menu, following links, getting back from links).

By mistake [...] I want to go to the top-right corner to the ☰ menu but it hits the Table of contents instead

But at this point, your finger is around the top right corner, near the x close button, so should be quite fast to close :)

@Frenzie
Copy link
Member

Frenzie commented Sep 15, 2018

Is there a rationale behind it?

@poire-z
Copy link
Contributor

poire-z commented Sep 15, 2018

If it = Swipe south , not much but:

  • for non full screen dialogs, I most often use Tap outside to close than the x button
  • on full screen dialogs, you can't Tap outside, so Swipe is the next easiest gesture
  • swipe left and right are used in these menus to change page - south and north are not
  • swipe south brings down the window
  • swipe south reminds me of the swipe from top of screen to bottom to close a full-screen app in Windows 8 & 10 in tablet mode (dunno if there's something similar in Android)

And it feels quite natural since I've been using it.

@Frenzie
Copy link
Member

Frenzie commented Sep 15, 2018

swipe left and right are used in these menus to change page - south and north are not

Up and down feel like basically the same thing though.

@ersi-dnd
Copy link
Author

@poire-z

But at this point, your finger is around the top right corner, near the x close button, so should be quite fast to close :)

One would assume so, but it does not work in reality. Just like it keeps hitting the TOC when I am aiming at ☰ , it also misses the x. Consistently so. At the same time, the Cancel or Dismiss buttons around the Go to page or other dialogs always react as expected, which is why I would like more buttons like this.

@ersi-dnd
Copy link
Author

ersi-dnd commented Oct 5, 2018

No, we don't have that. There's no way to change behaviour of gestures

In the latest git update I see that this has changed. Under Cogwheel > Device we now have Gesture manager and I can set a tap in corner to toggle night mode.

It (the Gesture manager) looks like something that would perhaps better be placed into Plugins menu, where the Frontlight gesture controller resides.

Besides, it would be nice if Frontlight gesture controller would, in addition to just toggling the light on-off, optionally open up the frontlight dialog. Just a setting to switch the gesture action between the toggle and the dialog.

Thank you for your great work!

@poire-z
Copy link
Contributor

poire-z commented Oct 16, 2018

@ersi-dnd : has the swipe down to close TOC helped getting out of it more easily?
Can #3997 be closed?

@ersi-dnd
Copy link
Author

@poire-z
Yes, it's a good solution. Thank you very much!

@ersi-dnd
Copy link
Author

And can the same or similar be implemented in history list?

@poire-z
Copy link
Contributor

poire-z commented Oct 17, 2018

That should already work.
In #4237:

Mostly all that have a close button at top right, that may be hard to reach or hit for some people:
TOC, Bookmarks, History, KeyValuePage (Book information, Statistics...), Book status...

@Frenzie
Copy link
Member

Frenzie commented Feb 23, 2019

@ersi-dnd I'll close this issue now since it looks resolved. Please feel free to reopen this one or to start a new issue.

@Frenzie Frenzie closed this as completed Feb 23, 2019
@ersi-dnd
Copy link
Author

Yes, it is resolved. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants