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

New MovableContainer: allow moving some widgets #3636

Merged
merged 1 commit into from Jan 29, 2018

Conversation

Projects
None yet
2 participants
@poire-z
Contributor

poire-z commented Jan 29, 2018

I wanted to be able to move some InfoMessage: often, when reading and wanting to do a dict or wiki lookup, I have the habbit of remembering the word, continue reading till I pass the 2/3 of the screen, and then do the lookup - so the InfoMessage doesn't pop on where I am reading, and I can still read a few lines while the lookup is happening.

Anyway, this PR adds a MovableContainer, and plugs it into a few widgets:

  • SkimToWidget (because in the middle of the screen is sometimes not the best place to see what's on the screen, and you can wish to move it to see some chapter title underneath it).
  • ReaderSearch (buttondialog) widget with the << < > >> buttons (for the same reason as SkimToWidget)
  • ButtonDialog/ButtonDialogTitle: the highlight 2x4 menu, the Hold on file menu, that you may wish to move to see again what's underneath.
  • InfoMessage & ConfirmBox: mostly for the lookups/Wiki save as EPUB, so you can move them out while waiting.
  • InputDialog: for consistency because why not?
  • DictQuickLookup: because sometimes, you may want to see the text around the word you just looked up.
  • TextViewer: could be useful with bookmarks to see the list while viewing one.

It also can set some alpha transparency to the subwidgets (I had in the past tried to plug an AlphaContainer to the SkimToWidget, but I always failed, and didn't really understood why - it was easier to add that to that MovableContainer). (Note that the existing alpha=0.5 in readersearch.lua has no widget in its hierachy that uses it, so it has no effect - but it will now if we put it back, 0.7 is nicer.)

I first had various options set for different widgets: alpha for SkimTo and Search, whether the move can push the widget off-screen (for the big DictQuickLookup), or whether it should be restricted to screen (for most others), but then decided to have a common behaviour for all of them, which is:

  • Move with Swipe: the widget will be constrained to screen borders (swipe is the quick natural way to move it quickly, with the nice side effect that there is a 0°/45°/90°... direction only, so it stays aligned)
  • Move with Hold and pan: the widget can overflow the borders, and be moved any direction.
  • Hold with no move: will reset the widget to its original position.
  • If the widget has not been moved or is already at its original position, Hold with no move will toggle between full opacity and 0.7 transparency.

So it shouldn't have any impact on our current habits, as we usually don't use these gestures with these widgets.
With the exception of DictQuickLookup, where Swipe left/right to change to next/prev dict used to work on all the screen. For move with swipe (so, starting from the title bar or the buttons) to work, I had to limit that to the definition area (which looks like the natural place for swiping and changing its content - that's where I naturally swipe for that). Dunno if it may bug some people. (The other Hold and Hold with pan on text to select still work).

In action (animated gif):

movable_widgets

If the widget has not been moved or is already at its original
position, Hold will toggle between full opacity and 0.7 transparency.
This container's content is expected to not change its width and height.

This comment has been minimized.

@Frenzie

Frenzie Jan 29, 2018

Member

While moving or ever?

This comment has been minimized.

@poire-z

poire-z Jan 29, 2018

Contributor

Not sure. I added that comment early, and may be it could work after I worked on the update region - possibly some side effects on the restrict to screen stuff - well, dunno if we have a real size changing widget to test it with :)

This comment has been minimized.

@Frenzie

Frenzie Jan 29, 2018

Member

I don't think we do. There are some things that superficially look like it (e.g., full Wikipedia) but nothing that actually does.

@Frenzie

lgtm

UIManager:setDirty("all", function()
local update_region = orig_dimen:combine(self.dimen)
logger.dbg("MovableContainer refresh region", update_region)
return "ui", update_region

This comment has been minimized.

@Frenzie

Frenzie Jan 29, 2018

Member

I haven't tested this on a device but isn't "ui" (in practice generally UPDATE_MODE_PARTIAL) a bit slow compared to "fast" (generally WAVEFORM_MODE_A2) or am I mistaken in that? We'd want to follow it with something like "ui" or "partial" in that case once the moving is over.

This comment has been minimized.

@poire-z

poire-z Jan 29, 2018

Contributor

Oh, I didn't mention it in the introduction, but this doesn't actually draw anything while moving (I don't really like that on eInk - and it was probably more complicated:) : it only draws the final position, when one releases its finger.
(It's not like page scrolling where I see how live drawing would matter even if I don't like it).
I use "ui", which is what is used in most InfoMessage&others' :onShow() (and not "partial" as this results in a full REAGL refresh on Kindle).

This comment has been minimized.

@Frenzie

Frenzie Jan 29, 2018

Member

Oh, I see. :-)

@Frenzie Frenzie merged commit 43a6cf4 into koreader:master Jan 29, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@poire-z poire-z deleted the poire-z:movable_widgets branch Jan 29, 2018

@poire-z poire-z referenced this pull request Jan 31, 2018

Closed

Fulltext search #3616

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment