Bug 90855 - DIALOG: Improve the 'Insert Bookmark' dialog
Summary: DIALOG: Improve the 'Insert Bookmark' dialog
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other All
: medium enhancement
Assignee: Jakub Trzebiatowski
URL:
Whiteboard: ToBeReviewed target:5.2.0
Keywords: difficultyInteresting, easyHack, skillCpp, topicUI
Depends on:
Blocks: Dialog Bookmarks
  Show dependency treegraph
 
Reported: 2015-04-25 03:59 UTC by Yousuf Philips (jay) (retired)
Modified: 2017-06-01 18:06 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
test document (15.36 KB, application/vnd.oasis.opendocument.text)
2016-03-12 01:07 UTC, Yousuf Philips (jay) (retired)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yousuf Philips (jay) (retired) 2015-04-25 03:59:52 UTC
The current 'Insert Bookmark' dialog does more than just insert a bookmark. It lists the available list of bookmark and also allows you to delete an existing bookmark, so i'd like to make the following change the dialog.

1) Change dialog title to 'Bookmarks'
2) Add a 'Go To' button that will jump to a selected existing bookmark
3) Change the bookmark list control into a control that has 3 columns - page number, bookmark id, bookmark text. Bookmark text would be the the first few words that appear after the beginning of the bookmark but would stop at a paragraph break.
Comment 1 Yousuf Philips (jay) (retired) 2015-04-25 04:02:02 UTC
Setting NEW as ux-advise enhancement.
Comment 2 Björn Michaelsen 2015-07-05 07:09:39 UTC
Hmm, wouldnt it be better to simplify the dialog to really just cover the "insert bookmark" usecase? We have the navigator prominently in the sidebar now, and it appears to me to be a much more natural way to manage/delete/edit existing bookmarks.
Comment 3 Yousuf Philips (jay) (retired) 2015-09-08 08:48:17 UTC
(In reply to Björn Michaelsen from comment #2)
> Hmm, wouldnt it be better to simplify the dialog to really just cover the
> "insert bookmark" usecase?

Unfortunately navigator is not a beginner friendly (aka Benjamin) interface, secondly there are users who wont use the sidebar due to screen resolution, thirdly we should have a regularly used feature like bookmark management in more than just the navigator.

> We have the navigator prominently in the sidebar
> now, and it appears to me to be a much more natural way to
> manage/delete/edit existing bookmarks.

Yes it would be great to have context menu entries for bookmarks for jump and delete. For edit and manage, did you want to create another dialog for bookmark management or were you thinking that you wanted it in as a sidebar tab?
Comment 4 Robinson Tryon (qubit) 2015-12-13 11:24:10 UTC Comment hidden (obsolete)
Comment 5 Samuel Mehrbrodt (allotropia) 2016-02-03 15:12:15 UTC
EasyHack:
The ui file is in sw/uiconfig/swriter/ui/insertbookmark.ui and the code in sw/source/ui/misc/bookmark.cxx .
Comment 6 Robinson Tryon (qubit) 2016-02-18 14:51:54 UTC Comment hidden (obsolete)
Comment 7 Jakub Trzebiatowski 2016-03-08 09:23:32 UTC
I want to work on it. Can I start? Is someone working on it already?
Comment 8 Samuel Mehrbrodt (allotropia) 2016-03-08 09:25:51 UTC
(In reply to Jakub Trzebiatowski from comment #7)
> I want to work on it. Can I start? Is someone working on it already?

Please go ahead. It's not assigned to anyone.
Comment 9 Jakub Trzebiatowski 2016-03-09 12:51:45 UTC
I have a question about MVC model.
I'm making a view control class that inherits from SvSimpleTable.
This control will be usefull only for bookmarks.

Can I put the logic of adding / removing bookmarks from internal container into this class?
Comment 10 Jakub Trzebiatowski 2016-03-09 18:37:14 UTC
Bookmarks are identified with name. Should name be used as bookmark id?
The Bookmark text can change. It needs to be updated every time dialog is opened.
Comment 11 Tomaz Vajngerl 2016-03-09 22:50:33 UTC
(In reply to Jakub Trzebiatowski from comment #9)
> I have a question about MVC model.
> I'm making a view control class that inherits from SvSimpleTable.
> This control will be usefull only for bookmarks.
> 
> Can I put the logic of adding / removing bookmarks from internal container
> into this class?

Yes, that should not be a problem.

(In reply to Jakub Trzebiatowski from comment #10)
> Bookmarks are identified with name. Should name be used as bookmark id?
> The Bookmark text can change. It needs to be updated every time dialog is
> opened.

As I said on IRC:
- user given bookmark name is "bookmark id"
- "bookmark text" is the text that the bookmark points to (show just a portion of it), it needs to be updated everytime the dialog opens.
Comment 12 Yousuf Philips (jay) (retired) 2016-03-10 11:23:07 UTC
Glad to see things are progressing for you Jakub. If you need any UI/UX help, you can find me in the libreoffice-design irc channel as jphilipz. As you are tackling this bookmark bug, you maybe be interested in looking at bug 45589.
Comment 14 Yousuf Philips (jay) (retired) 2016-03-12 01:07:49 UTC
Created attachment 123513 [details]
test document

Here is a test document with a bookmark at the beginning of a line, selecting text within a line and at the end of a line.
Comment 15 Jakub Trzebiatowski 2016-03-13 15:50:38 UTC
Should bookmark names be case sensitive?
Is bookmark with name "Bookmark" and "bookmark" allowed at once?
Comment 16 Yousuf Philips (jay) (retired) 2016-03-14 06:49:44 UTC
(In reply to Jakub Trzebiatowski from comment #15)
> Should bookmark names be case sensitive?
> Is bookmark with name "Bookmark" and "bookmark" allowed at once?

No they shouldnt be case sensitive, as they are currently, so yes it is allowed.
Comment 17 Commit Notification 2016-03-25 08:55:44 UTC
Jakub Trzebiatowski committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=08da15cabdcef60191f4ed98ed694eba3e35b5e1

tdf#90855 Improve the 'Insert Bookmark' dialog

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Miklos Vajna 2016-03-25 08:56:44 UTC
Samuel: the patch looked reasonable to me, so to help meeting the GSoC deadline, I've approved the patch. (You're the mentor for this easy hack, but I hope that's fine.)

Jay: is it expected that now insert -> bookmark has both an insert button that inserts the bookmark + closes the dialog and also a close one? It feels a bit unexpected to have the dialog closed when I did not press the close button. :-)
Comment 19 Yousuf Philips (jay) (retired) 2016-03-25 09:41:14 UTC
(In reply to Miklos Vajna from comment #18)
> Jay: is it expected that now insert -> bookmark has both an insert button
> that inserts the bookmark + closes the dialog and also a close one? It feels
> a bit unexpected to have the dialog closed when I did not press the close
> button. :-)

Miklos: As the dialog isnt modal, the insert button closes the dialog. For all non-insert interactions, the close button closes the dialog.
Comment 20 Commit Notification 2016-03-31 12:52:37 UTC
Jakub Trzebiatowski committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8d123bf1491bcc7415f4dde3ddd397a11146bb38

tdf#90855 add table header entries to resources

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 21 Commit Notification 2016-04-04 07:06:34 UTC
Jakub Trzebiatowski committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3dd9a3027d39ee9932b26abcde12363c881bff6a

tdf#90855 replace Dialog::Close with Dialog::EndDialog

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.