Merge lp://staging/~bratsche/oif/evince-drag-selection into lp://staging/~oif-team/oif/evince-gestures-trunk

Proposed by Cody Russell
Status: Needs review
Proposed branch: lp://staging/~bratsche/oif/evince-drag-selection
Merge into: lp://staging/~oif-team/oif/evince-gestures-trunk
Diff against target: 345 lines (+203/-24)
3 files modified
libview/ev-view.c (+131/-9)
libview/ev-view.h (+11/-0)
shell/ev-window.c (+61/-15)
To merge this branch: bzr merge lp://staging/~bratsche/oif/evince-drag-selection
Reviewer Review Type Date Requested Status
Duncan McGreggor (community) Needs Fixing
Review via email: mp+42545@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Duncan McGreggor (oubiwann) wrote :

I think that boulabiar mentioned this in another review, but the drag goes the wrong way for touch screen -- dragging left pans the substrate right, and vice versa.

Also, I noted that during pinch, one can't move the substrate, so we didn't get that for free. I'll update the related ticket.

review: Needs Fixing
Revision history for this message
Duncan McGreggor (oubiwann) wrote :

I'm trying to tap the icons in the tool bar (e.g., the previous and next buttons) and there's no response. Has that branch been merged yet? If not, disregard. If so, then there's a problem, 'case tap on the toolbar isn't working...

review: Needs Fixing
Revision history for this message
Duncan McGreggor (oubiwann) wrote :

Now, for the actual functionality implemented in this branch. I'm getting inaccurate selections of text. Here are the steps I'm taking:

 1. double tap to get selection mode
 2. drag my finger down to select text
 3. text selection is off by a little over a line in some areas, a few words in other areas, and several lines in others.

I thought this might be due to the finger overlapping the width of a line, so I zoomed in very large and tried it. In some instances, this actually made the results worse (the selection would jump several lines from where I actually touched).

review: Needs Fixing
Revision history for this message
Mohamed IKBEL Boulabiar (boulabiar) wrote :

> I think that boulabiar mentioned this in another review, but the drag goes the
> wrong way for touch screen -- dragging left pans the substrate right, and vice
> versa.

For this issue, it is corrected in this branch
https://code.launchpad.net/~boulabiar/evince/evince-drag-fixes

Revision history for this message
Duncan McGreggor (oubiwann) wrote :

Thanks, Ikbel. I'm assuming that even though you put this in the evince namespace, you actually branched from Cody's work, yes?

Cody, if you end up using this, please don't patch. Please merge Ikbel's branch to yours. This will preserve his contributions and history.

Thanks!

Unmerged revisions

4213. By Cody Russell

Remove unused commented code.

4212. By Cody Russell

Selection clearing on tap.

4211. By Cody Russell

Tap-and-drag selection

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches