Reader: rationalize "Back" key/action handling #6840
Conversation
Have ReaderBack be the sole handle of onBack. Add 4 mutually exclusive options for the Back key, to avoid ReaderLink and ReaderBack location stacks from interfering (ReaderBack's stack being always a superset of ReaderLink's stack). So, remove "Enable back history", which is replaced by Back option "Go to previous read page". Fix a few possible crashes and inconsistencies (when zoom, scroll or reflow have changed) with ReaderPaging and ReaderView when restoring previous page/view.
There may be a case to be made for "previous location" there, but no strong feelings about that (and no real first-hand experience either, as I've never actually used KOReader on a K4 ;)). |
| elseif #location_stack > 1 then | ||
| local saved_location = table.remove(location_stack) | ||
| -- Hook to events that do/may change page/view (more than one of these events | ||
| -- might be send on a single page turn/scroll, _addPreviousLocationToStack() |
There was a problem hiding this comment.
| -- might be send on a single page turn/scroll, _addPreviousLocationToStack() | |
| -- may be sent on a single page turn/scroll, _addPreviousLocationToStack() |
| else | ||
| logger.dbg("[ReaderBack] no location history, closing") | ||
| elseif back_in_reader == "previous_location" then | ||
| -- ReaderLink maintain its own location_stack of less frequent jumps |
There was a problem hiding this comment.
| -- ReaderLink maintain its own location_stack of less frequent jumps | |
| -- ReaderLink maintains its own location_stack of less frequent jumps |
| -- (links or TOC entries followed, skim document...) | ||
| if back_to_exit == "disable" then | ||
| -- Let ReaderLink always show its notification if empty | ||
| self.ui.link:onGoBackLink(true) -- show_notification_if_empty=true |
There was a problem hiding this comment.
Ooh, good idea of the Python-like named argument comment, makes those kind of tricks much more readable ;).
| -- of either the previous page or the current one). | ||
| UIManager:nextTick(self._addPreviousLocationToStackCallback) | ||
| end | ||
| self.back_resist = nil |
There was a problem hiding this comment.
I'm not super-fond of the variable name, but I don't really have any better idea either ;D.
I agree, so done. And having "previous location" as the default restores the ReaderLink "Back" handling I added for following links with keys (and when one has not followed any link or jumped around, it fallbacks to "Back to exit (prompt|always|disable)" - which should be fine. |
|
Latest commit according to #6798 (review) |
Have ReaderBack be the sole handler of onBack.
Add 4 mutually exclusive options for the Back key, to avoid ReaderLink and ReaderBack location stacks from interfering (ReaderBack's stack being always a superset of ReaderLink's stack).
So, remove "Enable back history", which is replaced by Back option "Go to previous read page".
Fix a few possible crashes and inconsistencies (when zoom, scroll or reflow have changed) with ReaderPaging and ReaderView when restoring previous context (page/view).
Some details about the issues in #6619 (comment) and follow up.
It can still be a bit of a mess (mostly, one back out of two not changing anything) in ReaderPaging, when switching some zoom/scroll/reflow settings. I tried and failed fixing more stuff in ReaderPaging/ReaderView, so I ended up being the less intrusive in them - at the expense of catching many events, some possibly uneeded/duplicating. But well...
The default for this new option is now the
Back to exitaction. No need to have "Go to previous read page" (old "Enable back history") be the default as most devices don't have a back key, and I think not many users might use the back action (which was quite confusing with its inconsitencies).I don't know if we need to have an other default for devices with hasKeys and not isTouch.
This change is