jangmarker added a comment.
In D21364#469111 <https://phabricator.kde.org/D21364#469111>, @davidhurka wrote: > Nice, makes the list more intuitive. :) (Didn’t test this) Thank you :-) Currently it looks like this: F6844393: image.png <https://phabricator.kde.org/F6844393> > Three questions/suggestions: > > - Assuming that captionForAnnotation() returns something more useful than “<annotation type> with Comment” (like “<annotation type>: <comment>”), should data() return an "okular" icon by default? If there is no data that can be shown in an icon, the horizontal space could be used for the comment instead. I think `captionForAnnotation()` does not include the comment's text currently, so this change should probably be done in another commit. I would personally also like it, if e.g. for "Highlights" of text it Okular could show the actual text which was highlighted. > - Maybe data() could return Annotation::icon(), where icon() is a virtual funtion which returns an appropriate, colored QIcon? I'm not against doing this, I'm just wondering which additional information the icon holds? Before this commit it did *always* show the Okular icon which does not add any information. It would make sense to show icons to indicate the annotation type, e.g. a different icon for highlight, freehand lines, underlines etc. And if these were colored according to the annotation's color, that would be perfect IMO. > - Weren’t the list items ordered by y-position once? I do not know, sorry. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D21364 To: jangmarker, #okular Cc: davidhurka, okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid
