Nice, makes the list more intuitive. :) (Didn’t test this)

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.
• Maybe data() could return Annotation::icon(), where icon() is a virtual funtion which returns an appropriate, colored QIcon?
• Weren’t the list items ordered by y-position once?

In D21364#469111, @davidhurka wrote:

Nice, makes the list more intuitive. :) (Didn’t test this)

Thank you :-) Currently it looks like this:

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.

In D21364#469159, @jangmarker wrote:

I think captionForAnnotation() does not include the comment's text currently, so this change should probably be done in another commit.

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.

That’s what I meant. :)

I would personally also like it, if e.g. for "Highlights" of text it Okular could show the actual text which was highlighted.

Makes sense. I. e. instead of showing the comment, because how often does one add a comment to e. g. a squiggle underline?

In D21364#469111, @davidhurka wrote:

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.

Thought about it: the okular icon might make sense. The stamp annotation is the only one which doesn’t use color, and coincidentally stamps an Okular icon. ;) But unfortunately, it has a color. What do you get in the tree view if you make some stamps?

In D21364#469215, @davidhurka wrote:

In D21364#469159, @jangmarker wrote:

I think captionForAnnotation() does not include the comment's text currently, so this change should probably be done in another commit.

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.

That’s what I meant. :)

I'm currently investigating whether I can make that work :-) That would be really cool.

I would personally also like it, if e.g. for "Highlights" of text it Okular could show the actual text which was highlighted.

Makes sense. I. e. instead of showing the comment, because how often does one add a comment to e. g. a squiggle underline?

In D21364#469111, @davidhurka wrote:

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.

Thought about it: the okular icon might make sense. The stamp annotation is the only one which doesn’t use color, and coincidentally stamps an Okular icon. ;) But unfortunately, it has a color. What do you get in the tree view if you make some stamps?

With the current code it's just showing the Okular icon as usual, because when I tried the color was invalid.

Now it looks like this:

A stamp looks like this:

I changed the color of this stamp to something blue, but the algorithm that's creating the icons does not use color for stamps, it just makes sure that the correct icon is shown (in the case of my stamp the Okular icon). This should probably be another patch then (although I don't feel like this is necessary, because the stamp in the document also has not visible color).

Why do you move EditAnnotToolDialog to GuiUtils? / How does the annotation tool bar (shown with F6) get its icons?

I’m not familar with the annotation implementation, but if you move this function arround, I would expect it as virtual function in Annotation, so every annotation can implement icon generation the correct way. (Don’t know how the toolbar would get its icons then...)

In D21364#469241, @jangmarker wrote:

Now it looks like this:

Looks ok, but if someone makes PageViewAnnotator::makeToolPixmap() return a QIcon(Engine) instead of a pixmap, it will look better.