GNOME Bugzilla – Bug 774018
Use I-beam for text markup annotations
Last modified: 2016-11-21 08:05:58 UTC
Selection of a text span is hard with how evince forces the use of the ADD cursor for _all_ annotation types upon selection and then a NORMAL cursor during dragging. I propose using an IBEAM cursor in this case. [771627][https://bugzilla.gnome.org/show_bug.cgi?id=771627] might be considered related
Created attachment 339204 [details] [review] use IBEAM for TEXT_MARKUP annotations Selection of text is hard when not using an I-beam cursor, this commit introduces using an I-beam for text-markup type annotations, as those are about marking a text selection.
An alternative would be to use a custom cursor (e.g. variant of I-Beam with added marker) that indicates that a text-markup operation is currently selected. We would need a custom cursor for this though and I don't know how the maintainers feel about adding more cursors, possible disruptions with OS cursor-themes and so on. Could be added later through separate patch though.
Review of attachment 339204 [details] [review]: Thanks for the patch, I think it's a good idea to use IBEAM for text highlighting. ::: libview/ev-view.c @@ +2112,3 @@ return; if (view->adding_annot_info.adding_annot && !view->adding_annot_info.annot) { I would change this, what we want is that cursor is IBEAM always when adding a text markup annotation, no matter if we already have an annot or not. So, I would change this ifs to do something like: if (view->adding_annot_info.adding_annot) { if (view->adding_annot_info.type == EV_ANNOTATION_TYPE_TEXT_MARKUP) change cursor to IBEAM else if (!view->adding_annot_info.annot) change cursor to ADD return; } @@ +3418,3 @@ + if (view->adding_annot_info.type != EV_ANNOTATION_TYPE_TEXT_MARKUP) { + ev_view_set_cursor (view, EV_VIEW_CURSOR_NORMAL); + } And here, instead of adding the comment and making text markup a special case, simply call ev_view_handle_cursor_over_xy() and the cursor will be updated properly.
Created attachment 339765 [details] [review] use IBEAM cursor for TEXT_MARKUP annotations Text markup annotations are hard with the cursor used so far for annotations in general (ADD), using I-Beam is better for text selection. This patch further removes checking the currently set cursor type in `ev_view_handle_cursor_over_xy` while setting it, at least for creating annotations, as this is taken care of by `ev_view_set_cursor` already. Setting the cursor to NORMAL in `ev_view_create_annotation` is removed, as it seems superfluous and introduces wrong behavior during dragging (i.e. while creating a text markup annotation), and the cursor type is handled by `ev_view_handle_cursor_over_xy` already.
Comment on attachment 339765 [details] [review] use IBEAM cursor for TEXT_MARKUP annotations Pushed, thank you!