Created attachment 242706 [details] [review]
patch adding tooltips

Review of attachment 242706 [details] [review]:

Thanks for the patch! Looks mostly good, with some comments.

::: src/preview.js
@@ +590,3 @@
                                       valign: Gtk.Align.CENTER
                                     });
+        button.set_tooltip_text('Contents');

You can set the tooltip text as part of the constructor above, like

let b = new Gtk.Button({ tooltip_text: 'text', ... });

The text there should also be marked for translation using gettext(). For brevity, GNOME has a convention of defining a "_()" macro to mark string for translation (see the imports section at the top of preview.js). In Documents we also use double quotes for translatable strings inside _().

Finally, I think "Bookmarks" would be a better name - the table of contents links are essentially glorified bookmarks.

@@ +599,3 @@
                                         valign: Gtk.Align.CENTER
                                       });
+        button.set_tooltip_text('Add bookmark');

I'd call it "Bookmark this page"

Cool! I'll fix it and send another patch.

Thanks for your 'teachingfull' comment!

Created attachment 242752 [details] [review]
modified after hints received

Review of attachment 242752 [details] [review]:

Looks good to me

Comment on attachment 242752 [details] [review]
modified after hints received

Err, actually you should squash the two patches together into a single one.

Created attachment 242753 [details] [review]
One Patch to rule them all!

Review of attachment 242753 [details] [review]:

Yes.

Pushed this to git master now, thanks!