View List of Bookmarks and add Metadata to them

[Daniel Bedrenko]

Firstly, a huge thank you for this light PDF reader--I love it!

However, a very important feature for me that this reader lacks is the management of bookmarks. Specifically:

-Support naming bookmarks
-Support adding a description to a bookmark
-Have a dock (like the Thumbnails and Outline) that lists all bookmarks for the file, including page number, bookmark name and the truncated description. Clicking on a bookmark in this list, takes you to the bookmarked page.

This is essential for instructional, academic and reference books--and the like--where you have to navigate around the book often, unlike a novel which you generally read back-to-front. Imagine having more than 10 bookmarks, and trying to find a particular piece of information you bookmarked; having only the page-number to go by is not enough so you must keep in your head what each bookmark points to, and, when you forget this, you have to jump between bookmarks until you find it.

See attached a screenshot of this functionality in obsolete kpdf2 (Okular's predecessor).

[Adam Reichold (adamreichold)]

Hello,

thank you for your suggestion. Named resp. labelled bookmarks are available in version 0.4.7, more elaborate bookmarking management mechanisms are for the moment considered out of scope w.r.t. intended program complexity and available developer time. Sorry, I'll leave the bug report as triaged if I or someone else find time to pick it up in the future.

Best regards, Adam.

[Razi Alavizadeh (srazi)]

Hello
I also need a bookmark sidebar and if I have time I'll implement it.
But what GUI option is better?
1- dock show bookmarks of current tab in a listview
2- dock show all bookmarks in a treeview, for each document there's a parent item that contains document's bookmarks as its children.
3- both 1 & 2 and add an option in settings window to choose between view mode 1 or 2.
4- dock has a splitter that contains a listview and a treeview, in other words, it shows bookmarks of current tab and all bookmarks in the same time

Best Regards,
Razi.

[Daniel Bedrenko (dbedrenko)]

Personally, I would pick 1. To have all bookmarks (from all tabs) in a treeview is not as intuitive as showing the bookmarks for the current file/tab. qpdfview seems to be a light-weight PDF viewer, so it doesn't seem necessary to add a feature for cross-referencing. Having a tree view is also more programming work, and I don't think the benefit is enough--it's quite small for my use-cases (but maybe other people have other use-cases to me).

To have all bookmarks available implies that the user would be switching back and forth between this file and that. If it was me, if I needed to check a bookmark in another document, I'd just switch between tabs (it's not hard with Ctrl+Tab).

Thank you for your work.

[Adam Reichold (adamreichold)]

Hello Razi and Aeou,

I also think that anything but alternative 1 is cluttered and does not integrate nicely with the tabs as a means of switching documents. I also would not go beyond a label and a textual comment as meta-data.

In any case, I would recommend refactoring the bookmarks support to use a proper item model as a prerequisite since the model-view approach will make is achievable to keep bookmarks menu and dock synchronized. This would also remove the design problem of the BookmarkMenu class currently providing both presentation and storage for the bookmarks.

Best regards, Adam.

[Razi Alavizadeh (srazi)]

Hello Adam and Aeou,

Thanks for your comments.
I implemented it (not yet completed), available here [1] and as you suggested I chose option 1.

@Adam: For applying model approach to bookmark menu I have two idea:
1- (seems easier to me) connect aboutToShow signal of bookmark-menu to a slot and compute menu based on model.
2- (seems needs more work) re-implement BookmarkMenu in a way that it works similar to view classes in Qt code (i.e. connecting all signals of model to slots of BookmarkMenu and ...)

What's your idea?

[1] http://bazaar.launchpad.net/~srazi/qpdfview/bookmarks-dock/revision/1660

Best Regards,
Razi.

[Adam Reichold (adamreichold)]

Hello again,

Am 05.09.2014 um 17:14 schrieb S. Razi Alavizadeh:
> Hello Adam and Aeou,
>
> Thanks for your comments.
> I implemented it (not yet completed), available here [1] and as you suggested I chose option 1.
>
> @Adam: For applying model approach to bookmark menu I have two idea:
> 1- (seems easier to me) connect aboutToShow signal of bookmark-menu to a slot and compute menu based on model.
> 2- (seems needs more work) re-implement BookmarkMenu in a way that it works similar to view classes in Qt code (i.e. connecting all signals of model to slots of BookmarkMenu and ...)
>
> What's your idea?

From your description and since a pop-up menu is only a temporary view
where you can't really interact with the model continuously, option 1
actually sounds like a nice idea to evade the full complexity of
implementing a view. So if you can implement it cleanly, I am for option 1.

> [1] http://bazaar.launchpad.net/~srazi/qpdfview/bookmarks-
> dock/revision/1660

Will try it out as soon as I find the time.

> Best Regards,
> Razi.

Best regards, Adam.

[Razi Alavizadeh (srazi)]

Hello,
The branch was updated to final code. [1]

[1] http://bazaar.launchpad.net/~srazi/qpdfview/bookmarks-dock/changes

[Adam Reichold (adamreichold)]

Hello Razi,

thank you for your contribution. I merged after minimal changes:
* Some nit pickings like the placement of ampersands.
* One has to check the column count before using QHeaderView::setSectionResizeMode in Qt5.

Best regards, Adam.

[Adam Reichold (adamreichold)]

Hello again,

I just pushed trunk revision 1665 which extends the bookmarks with comment and modification date and time fields (bumping the bookmark format to v3) and adds a little dialog to edit these properties that is accessible via the context menu of the bookmarks dock. Please test and report your findings here.

Best regards, Adam.

[Razi Alavizadeh (srazi)]

Hello Adam
> thank you for your contribution. I merged after minimal changes:
> * Some nit pickings like the placement of ampersands.
> * One has to check the column count before using QHeaderView::setSectionResizeMode in Qt5.

Thanks for your points and also for improving my code. :-)

> I just pushed trunk revision 1665 which extends the bookmarks with comment and modification date and time fields (bumping the bookmark format to v3) and adds a little dialog to edit these properties that is accessible via the context menu of the bookmarks dock. Please test and report your findings here.

Great! I suggest that by using a delegete comment be visible under label of bookmark. (like attached screenshot by Aeou)

Also there is a compile issue with MSVC++ that can be fixed by following patch:

=== modified file sources/mainwindow.h
--- sources/mainwindow.h 2014-09-07 16:15:24 +0000
+++ sources/mainwindow.h 2014-09-07 19:27:39 +0000
@@ -26,6 +26,7 @@
 #define MAINWINDOW_H

 #include <QMainWindow>
+#include <QHash>

 #ifdef WITH_DBUS

@@ -42,6 +43,7 @@
 class QStandardItemModel;
 class QTableView;
 class QWidgetAction;
+class QDateTime;

 #include "global.h"

Best Regards,
Razi.

[Adam Reichold (adamreichold)]

Am 07.09.2014 um 22:00 schrieb S. Razi Alavizadeh:
> Hello Adam
>> thank you for your contribution. I merged after minimal changes:
>> * Some nit pickings like the placement of ampersands. * One has
>> to check the column count before using
>> QHeaderView::setSectionResizeMode in Qt5.
>
> Thanks for your points and also for improving my code. :-)

Yeah, sorry about all the rewriting. Let's call it maintainer's
privilege...

>> I just pushed trunk revision 1665 which extends the bookmarks
>> with
> comment and modification date and time fields (bumping the
> bookmark format to v3) and adds a little dialog to edit these
> properties that is accessible via the context menu of the bookmarks
> dock. Please test and report your findings here.
>
> Great! I suggest that by using a delegete comment be visible under
> label of bookmark. (like attached screenshot by Aeou)

I left it in the tool tip role for the time being as I am more
concerned about encapsulating all the ad-hoc usage of
QStandardItemModel (I know it has infested a lot of the code base) by
thinking about how to best introduce a BookmarkModel class...

> Also there is a compile issue with MSVC++ that can be fixed by
> following patch:

Pushed.

Best regards, Adam.

> === modified file sources/mainwindow.h --- sources/mainwindow.h
> 2014-09-07 16:15:24 +0000 +++ sources/mainwindow.h 2014-09-07
> 19:27:39 +0000 @@ -26,6 +26,7 @@ #define MAINWINDOW_H
>
> #include <QMainWindow> +#include <QHash>
>
> #ifdef WITH_DBUS
>
> @@ -42,6 +43,7 @@ class QStandardItemModel; class QTableView; class
> QWidgetAction; +class QDateTime;
>
> #include "global.h"
>
>
> Best Regards, Razi.
>

[Adam Reichold (adamreichold)]

Hello again,

Am 07.09.2014 um 22:00 schrieb S. Razi Alavizadeh:
> Hello Adam
>> thank you for your contribution. I merged after minimal changes:
>> * Some nit pickings like the placement of ampersands.
>> * One has to check the column count before using QHeaderView::setSectionResizeMode in Qt5.
>
> Thanks for your points and also for improving my code. :-)
>
>> I just pushed trunk revision 1665 which extends the bookmarks with
> comment and modification date and time fields (bumping the bookmark
> format to v3) and adds a little dialog to edit these properties that is
> accessible via the context menu of the bookmarks dock. Please test and
> report your findings here.
>
> Great! I suggest that by using a delegete comment be visible under label
> of bookmark. (like attached screenshot by Aeou)
>
> Also there is a compile issue with MSVC++ that can be fixed by following
> patch:

I now refactored the bookmarks into a separate model class with central
registry using absolute file paths. Could you try to test whether I
broke compilation or functionality? Thanks!

Best regards, Adam.

> === modified file sources/mainwindow.h
> --- sources/mainwindow.h 2014-09-07 16:15:24 +0000
> +++ sources/mainwindow.h 2014-09-07 19:27:39 +0000
> @@ -26,6 +26,7 @@
> #define MAINWINDOW_H
>
> #include <QMainWindow>
> +#include <QHash>
>
> #ifdef WITH_DBUS
>
> @@ -42,6 +43,7 @@
> class QStandardItemModel;
> class QTableView;
> class QWidgetAction;
> +class QDateTime;
>
> #include "global.h"
>
>
> Best Regards,
> Razi.
>

[Razi Alavizadeh (srazi)]

Hello Adam,
> I now refactored the bookmarks into a separate model class with central
> registry using absolute file paths. Could you try to test whether I
> broke compilation or functionality? Thanks!
Edit bookmark duplicates bookmark.

Best Regards,
Razi.

2014-09-08 23:36 GMT+04:30 Adam Reichold <email address hidden>:

> Hello again,
>
> Am 07.09.2014 um 22:00 schrieb S. Razi Alavizadeh:
> > Hello Adam
> >> thank you for your contribution. I merged after minimal changes:
> >> * Some nit pickings like the placement of ampersands.
> >> * One has to check the column count before using
> QHeaderView::setSectionResizeMode in Qt5.
> >
> > Thanks for your points and also for improving my code. :-)
> >
> >> I just pushed trunk revision 1665 which extends the bookmarks with
> > comment and modification date and time fields (bumping the bookmark
> > format to v3) and adds a little dialog to edit these properties that is
> > accessible via the context menu of the bookmarks dock. Please test and
> > report your findings here.
> >
> > Great! I suggest that by using a delegete comment be visible under label
> > of bookmark. (like attached screenshot by Aeou)
> >
> > Also there is a compile issue with MSVC++ that can be fixed by following
> > patch:
>
> I now refactored the bookmarks into a separate model class with central
> registry using absolute file paths. Could you try to test whether I
> broke compilation or functionality? Thanks!
>
> Best regards, Adam.
>
> > === modified file sources/mainwindow.h
> > --- sources/mainwindow.h 2014-09-07 16:15:24 +0000
> > +++ sources/mainwindow.h 2014-09-07 19:27:39 +0000
> > @@ -26,6 +26,7 @@
> > #define MAINWINDOW_H
> >
> > #include <QMainWindow>
> > +#include <QHash>
> >
> > #ifdef WITH_DBUS
> >
> > @@ -42,6 +43,7 @@
> > class QStandardItemModel;
> > class QTableView;
> > class QWidgetAction;
> > +class QDateTime;
> >
> > #include "global.h"
> >
> >
> > Best Regards,
> > Razi.
> >
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1267266
>
> Title:
> View List of Bookmarks and add Metadata to them
>
> Status in qpdfview:
> Fix Committed
>
> Bug description:
> Firstly, a huge thank you for this light PDF reader--I love it!
>
> However, a very important feature for me that this reader lacks is the
> management of bookmarks. Specifically:
>
> -Support naming bookmarks
> -Support adding a description to a bookmark
> -Have a dock (like the Thumbnails and Outline) that lists all bookmarks
> for the file, including page number, bookmark name and the truncated
> description. Clicking on a bookmark in this list, takes you to the
> bookmarked page.
>
> This is essential for instructional, academic and reference books--and
> the like--where you have to navigate around the book often, unlike a
> novel which you generally read back-to-front. Imagine having more than
> 10 bookmarks, and trying to find a particular piece of information you
> bookmarked; having only the page-number to go by is not enough so you
> must keep in your head what each bookmark points to, and, when you
> forget this, you have to jump betw...

[Razi Alavizadeh (srazi)]

Hello again,
Also on removing bookmark it should check if model's list of bookmark is
empty then it forgets path of the model.

Best Regards,
Razi.

2014-09-09 0:21 GMT+04:30 Seyyed Razi Alavizadeh <email address hidden>:

> Hello Adam,
> > I now refactored the bookmarks into a separate model class with central
> > registry using absolute file paths. Could you try to test whether I
> > broke compilation or functionality? Thanks!
> Edit bookmark duplicates bookmark.
>
> Best Regards,
> Razi.
>
> 2014-09-08 23:36 GMT+04:30 Adam Reichold <email address hidden>:
>
>> Hello again,
>>
>> Am 07.09.2014 um 22:00 schrieb S. Razi Alavizadeh:
>> > Hello Adam
>> >> thank you for your contribution. I merged after minimal changes:
>> >> * Some nit pickings like the placement of ampersands.
>> >> * One has to check the column count before using
>> QHeaderView::setSectionResizeMode in Qt5.
>> >
>> > Thanks for your points and also for improving my code. :-)
>> >
>> >> I just pushed trunk revision 1665 which extends the bookmarks with
>> > comment and modification date and time fields (bumping the bookmark
>> > format to v3) and adds a little dialog to edit these properties that is
>> > accessible via the context menu of the bookmarks dock. Please test and
>> > report your findings here.
>> >
>> > Great! I suggest that by using a delegete comment be visible under label
>> > of bookmark. (like attached screenshot by Aeou)
>> >
>> > Also there is a compile issue with MSVC++ that can be fixed by following
>> > patch:
>>
>> I now refactored the bookmarks into a separate model class with central
>> registry using absolute file paths. Could you try to test whether I
>> broke compilation or functionality? Thanks!
>>
>> Best regards, Adam.
>>
>> > === modified file sources/mainwindow.h
>> > --- sources/mainwindow.h 2014-09-07 16:15:24 +0000
>> > +++ sources/mainwindow.h 2014-09-07 19:27:39 +0000
>> > @@ -26,6 +26,7 @@
>> > #define MAINWINDOW_H
>> >
>> > #include <QMainWindow>
>> > +#include <QHash>
>> >
>> > #ifdef WITH_DBUS
>> >
>> > @@ -42,6 +43,7 @@
>> > class QStandardItemModel;
>> > class QTableView;
>> > class QWidgetAction;
>> > +class QDateTime;
>> >
>> > #include "global.h"
>> >
>> >
>> > Best Regards,
>> > Razi.
>> >
>>
>> --
>> You received this bug notification because you are a bug assignee.
>> https://bugs.launchpad.net/bugs/1267266
>>
>> Title:
>> View List of Bookmarks and add Metadata to them
>>
>> Status in qpdfview:
>> Fix Committed
>>
>> Bug description:
>> Firstly, a huge thank you for this light PDF reader--I love it!
>>
>> However, a very important feature for me that this reader lacks is the
>> management of bookmarks. Specifically:
>>
>> -Support naming bookmarks
>> -Support adding a description to a bookmark
>> -Have a dock (like the Thumbnails and Outline) that lists all bookmarks
>> for the file, including page number, bookmark name and the truncated
>> description. Clicking on a bookmark in this list, takes you to the
>> bookmarked page.
>>
>> This is essential for instructional, academic and reference books--and
>> the like--where you have to navigate around the book often, unlike a
>> nove...

[Adam Reichold (adamreichold)]

Hello again,

Am 09.09.2014 um 00:51 schrieb S. Razi Alavizadeh:
> Hello again,
> Also on removing bookmark it should check if model's list of bookmark is
> empty then it forgets path of the model.

Both issues (the other was bookmarks being duplicate because of iterator
snafus) should be fixed now.

(Btw., I also think we now have accumulated enough changes to warrant a
0.4.12 release and would aim for stabilization, translation and code
review for the coming two weeks and plan a release of 0.4.12 or maybe
0.4.12beta1 on 2014-09-21.)

Best regards, Adam.

> Best Regards,
> Razi.
>
> 2014-09-09 0:21 GMT+04:30 Seyyed Razi Alavizadeh
> <email address hidden>:
>
>> Hello Adam,
>>> I now refactored the bookmarks into a separate model class with central
>>> registry using absolute file paths. Could you try to test whether I
>>> broke compilation or functionality? Thanks!
>> Edit bookmark duplicates bookmark.
>>
>> Best Regards,
>> Razi.
>>
>> 2014-09-08 23:36 GMT+04:30 Adam Reichold <email address hidden>:
>>
>>> Hello again,
>>>
>>> Am 07.09.2014 um 22:00 schrieb S. Razi Alavizadeh:
>>>> Hello Adam
>>>>> thank you for your contribution. I merged after minimal changes:
>>>>> * Some nit pickings like the placement of ampersands.
>>>>> * One has to check the column count before using
>>> QHeaderView::setSectionResizeMode in Qt5.
>>>>
>>>> Thanks for your points and also for improving my code. :-)
>>>>
>>>>> I just pushed trunk revision 1665 which extends the bookmarks with
>>>> comment and modification date and time fields (bumping the bookmark
>>>> format to v3) and adds a little dialog to edit these properties that is
>>>> accessible via the context menu of the bookmarks dock. Please test and
>>>> report your findings here.
>>>>
>>>> Great! I suggest that by using a delegete comment be visible under label
>>>> of bookmark. (like attached screenshot by Aeou)
>>>>
>>>> Also there is a compile issue with MSVC++ that can be fixed by following
>>>> patch:
>>>
>>> I now refactored the bookmarks into a separate model class with central
>>> registry using absolute file paths. Could you try to test whether I
>>> broke compilation or functionality? Thanks!
>>>
>>> Best regards, Adam.
>>>
>>>> === modified file sources/mainwindow.h
>>>> --- sources/mainwindow.h 2014-09-07 16:15:24 +0000
>>>> +++ sources/mainwindow.h 2014-09-07 19:27:39 +0000
>>>> @@ -26,6 +26,7 @@
>>>> #define MAINWINDOW_H
>>>>
>>>> #include <QMainWindow>
>>>> +#include <QHash>
>>>>
>>>> #ifdef WITH_DBUS
>>>>
>>>> @@ -42,6 +43,7 @@
>>>> class QStandardItemModel;
>>>> class QTableView;
>>>> class QWidgetAction;
>>>> +class QDateTime;
>>>>
>>>> #include "global.h"
>>>>
>>>>
>>>> Best Regards,
>>>> Razi.
>>>>
>>>
>>> --
>>> You received this bug notification because you are a bug assignee.
>>> https://bugs.launchpad.net/bugs/1267266
>>>
>>> Title:
>>> View List of Bookmarks and add Metadata to them
>>>
>>> Status in qpdfview:
>>> Fix Committed
>>>
>>> Bug description:
>>> Firstly, a huge thank you for this light PDF reader--I love it!
>>>
>>> However, a very important feature for me that this reader lacks is the
>>> management of bookmarks. Specifically:
>>...

[Razi Alavizadeh (srazi)]

Hello again,
> Both issues (the other was bookmarks being duplicate because of iterator
> snafus) should be fixed now.
Thanks, they were fixed now.
Also as a small fix please set "QAbstractItemView::SingleSelection" for m_bookmarksView selection mode.

> (Btw., I also think we now have accumulated enough changes to warrant a
> 0.4.12 release and would aim for stabilization, translation and code
> review for the coming two weeks and plan a release of 0.4.12 or maybe
> 0.4.12beta1 on 2014-09-21.)
OK, great!

Best Regards,
Razi.

[Adam Reichold (adamreichold)]

Am 09.09.2014 um 22:20 schrieb S. Razi Alavizadeh:
> Hello again,
>> Both issues (the other was bookmarks being duplicate because of iterator
>> snafus) should be fixed now.
> Thanks, they were fixed now.
> Also as a small fix please set "QAbstractItemView::SingleSelection" for m_bookmarksView selection mode.

Done. Thanks!

>> (Btw., I also think we now have accumulated enough changes to warrant a
>> 0.4.12 release and would aim for stabilization, translation and code
>> review for the coming two weeks and plan a release of 0.4.12 or maybe
>> 0.4.12beta1 on 2014-09-21.)
> OK, great!
>
> Best Regards,
> Razi.
>