D15580: [WIP] New annotation toolbar

simgunz added a comment. In D15580#554104 , @davidhurka wrote: > In D15580#554079 , @simgunz wrote: > > > Should I configure the toolbar so that if the user changes to "text alongside icons" the

D15580: [WIP] New annotation toolbar

simgunz added a comment. Regarding unchecking the mouse mode actions when an annotation is selected. As for now I think this is extremely complex to implement. I suggest leaving it as is for now, and once Qt 5.14 is out we can use the new option that allows unchecking all actions in a

D15580: [WIP] New annotation toolbar

simgunz added a comment. Is the icon fine for the toolbar hide/close button? Do you have a better suggestion? F7674605: Screenshot_20191027_092856.png *Hide toolbar button* REPOSITORY R223 Okular REVISION DETAIL

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 68813. simgunz marked 4 inline comments as done. simgunz added a comment. - Add action to hide the annotation toolbar - Preserve user-defined annotation tools - Remove unsueful initialization REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE

D15580: [WIP] New annotation toolbar

simgunz added inline comments. INLINE COMMENTS > davidhurka wrote in shell.cpp:370 > >> @simgunz just wrote: > > > > https://phabricator.kde.org/D15580#544534 > > That’s what I thought about. :) Initially I put that action in `part.rc`, while now I moved it to `shell.rc`. This makes more

D15580: [WIP] New annotation toolbar

ngraham added a comment. In D15580#554079 , @simgunz wrote: > I have never seen a close button on a standard toolbar. Are you sure it is a correct design pattern? This isn't a standard toolbar; it's a toolbar that can be shown and

D15580: [WIP] New annotation toolbar

davidhurka added inline comments. INLINE COMMENTS > davidhurka wrote in shell.cpp:370 > I remember you had struggle to find the annotation toolbar when you tried to > show and hide it. Is this code related to the problem and did you solve the > problem? >> @simgunz just wrote: > >

D15580: [WIP] New annotation toolbar

simgunz added a comment. >> I think there's only one remaining thing I noticed, then I'm ready to give it a UI stamp of approval: the toolbar should have a close button on the far-right side (you can use an expanding spacer to position it there) so people don't have to go up to the menubar

D15580: [WIP] New annotation toolbar

davidhurka added a comment. In D15580#554079 , @simgunz wrote: > Should I configure the toolbar so that if the user changes to "text alongside icons" the toolbar appears like this? > (the user can then expand the remaining icon-only buttons

D15580: [WIP] New annotation toolbar

simgunz added a comment. In D15580#553421 , @ngraham wrote: > All right, that works! > > I think there's only one remaining thing I noticed, then I'm ready to give it a UI stamp of approval: the toolbar should have a close button on the

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 68768. simgunz added a comment. - Warn user that stamps in PDF are an experimental feature REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=68650=68768 BRANCH new-annotation-toolbar_ToggleActionMenu REVISION

D15580: [WIP] New annotation toolbar

ngraham added a comment. All right, that works! I think there's only one remaining thing I noticed, then I'm ready to give it a UI stamp of approval: the toolbar should have a close button on the far-right side (you can use an expanding spacer to position it there) so people don't have

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 68650. simgunz added a comment. - Show annotation toolbar when quick annotation is selected REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=68649=68650 BRANCH new-annotation-toolbar_ToggleActionMenu REVISION

D15580: [WIP] New annotation toolbar

simgunz added a comment. In D15580#552773 , @ngraham wrote: > In D15580#552455 , @simgunz wrote: > > > When an annotation is selected its annotation toolbar button is checked and Browse Mode is

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 68649. simgunz added a comment. - Rename action: pin > keep active - Add action to quick annotation menu to show annotation toolbar REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=68493=68649 BRANCH

D15580: [WIP] New annotation toolbar

simgunz added a comment. > Maybe in a KMessageWidget that goes across the top of the view, like other notes that are presented like this. KMessageWidget does not have a mechanism to dismiss it forever. I think that showing it each time a user uses the stamp annotation is going to become

D15580: [WIP] New annotation toolbar

ngraham added a comment. In D15580#552455 , @simgunz wrote: > When an annotation is selected its annotation toolbar button is checked and Browse Mode is checked, exactly as in the current Okular version. > When you select a quick annotation

D15580: [WIP] New annotation toolbar

simgunz added a comment. In D15580#552362 , @ngraham wrote: > This is super fantastic. > > I have a few more observations from use: > > - When using one of the quick annotations, the Quick Annotations button should have a checked state

D15580: [WIP] New annotation toolbar

ngraham added a comment. In D15580#552381 , @davidhurka wrote: > Yeah, but [_] Annotation Toolbar would be more natural, I think. At least for users who figured out the meaning of “toolbar”. That's a good idea. It would be: `[ ] Show

D15580: [WIP] New annotation toolbar

davidhurka added a comment. Didn’t yet test this, but I like some of Nate’s ideas. In D15580#552362 , @ngraham wrote: > This is super fantastic. > > I have a few more observations from use: > > - Maybe add a new menu item at the

D15580: [WIP] New annotation toolbar

ngraham added a comment. This is super fantastic. I have a few more observations from use: - When using one of the quick annotations, the Quick Annotations button should have a checked state so you can tell that one of its tools is active (otherwise the previously-active toolbar

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 68493. simgunz added a comment. - Set toolbar default to icononly REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=68372=68493 BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL

D15580: [WIP] New annotation toolbar

ngraham added a comment. Those color pickers are great. Can we get the tool buttons moved back to being icons-only so they'll fit in one row for an un-maximized window? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc:

D15580: [WIP] New annotation toolbar

simgunz added dependencies: D21755: [RFC] Replace ToolAction by a more universal “ToggleActionMenu”, D21971: [DEMO] Enhance ToggleActionMenu with ImplicitDefaultAction mode.. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc:

D15580: [WIP] New annotation toolbar

simgunz added a subscriber: trickyricky26. simgunz added a comment. The toolbar is almost 100% ready from my side. What is left to do: - The opacity icon is rendered incorrectly but it is going to be fixed soon by @trickyricky26. - There are two TODO not yet fixed in T8076

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 68372. simgunz added a comment. - Use new draw-arrow icon - Move bookmark and pin to the right of the toolbar - Avoid foreach - Fix reference arguments - Use new edit-opacity icon - Remove support for inline note text color (too messy for now)

D15580: [WIP] New annotation toolbar

ngraham edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc: simgunz, ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, johnzh, andisa, siddharthmanthan, maguirre,

D15580: [WIP] New annotation toolbar

simgunz added a comment. > I think this is ok. Two wishes: > > - The toolbar button shows a checked state, when the fill color is se As it is now that button already displays a rectangle with the currect color (or nothing if no fill color). That should be enough to show the user the

D15580: [WIP] New annotation toolbar

davidhurka added a comment. In D15580#546492 , @ngraham wrote: > Typically the color chooser includes a "transparent" item. Also a bit complicated for me as user. :) In D15580#546395 ,

D15580: [WIP] New annotation toolbar

simgunz added a comment. In D15580#546492 , @ngraham wrote: > Typically the color chooser includes a "transparent" item. The things are trickier here. In Okular the opacity of the annotation is common to the whole annotation, i.e. we

D15580: [WIP] New annotation toolbar

ngraham added a comment. Typically the color chooser includes a "transparent" item. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc: simgunz, ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel,

D15580: [WIP] New annotation toolbar

simgunz added a comment. Currently once the inner color of an annotation (e.g. rectangle) has been selected, there is no way to remove it (making the inner part of the annotation transparent) using the toolbar. The only way is through `Adavenced Settings`. What do you think is the best

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 67832. simgunz added a comment. - Fix stamp not deselected with Pin unchecked - Move weird stamps to the bottom of the list - Fix cannot change text color of typewriter and inline note - Make stamp tool Id a static const - Make default stamps a

D15580: [WIP] New annotation toolbar

simgunz edited the summary of this revision. simgunz edited the test plan for this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc: simgunz, ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger,

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 67744. simgunz added a comment. - Use new highlighter icon REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=67589=67744 BRANCH annotation-toolbar-refactor REVISION DETAIL https://phabricator.kde.org/D15580

D15580: [WIP] New annotation toolbar

simgunz marked 2 inline comments as done. simgunz added a comment. I need some input on two things left to be done: 1. When a color picker is open and the user presses `Esc`, the color picker modal dialog is closed but the currently selected annotation action is deselected when it

D15580: [WIP] New annotation toolbar

simgunz marked 4 inline comments as done. simgunz added inline comments. INLINE COMMENTS > davidhurka wrote in toolaction.cpp:71 > The tooltip should become visible when you hover the toolbar button with the > three selection tools. It’s in the main toolbar by default. See also D21622 >

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 67589. simgunz added a comment. - Make ktoggleannotationaction to be owned by shell.cpp - Add hack to allow respecting the default toolbars layout REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=67517=67589

D15580: [WIP] New annotation toolbar

ngraham added a comment. Yeah. `draw-highlight` is coming too in D24493 . With the better icons, maybe we can move the inviditual tools back to being icons-only to save a ton of space, and then everything will fit into a toolbar when the window isn't

D15580: [WIP] New annotation toolbar

simgunz added a comment. I have refactored and cleaned the code. There are two tricky TODO missing. It seems that also the icons are coming. I think we are almost there. Once I fixed the TODO I'll test this further updating the test plan. F7549017: Screenshot_20191008_182346.png

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 67517. simgunz added a comment. - Rename favorite > quick in settings - Refactor - Clean pageviewutils - Increase part.rc version - Improve tools defaults - Clean code - Remove unused method - Rename methods - Rename, move and comment

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 67036. simgunz added a comment. - Merge remote-tracking branch 'origin/master' into new-annotation-toolbar_ToggleActionMenu - Fix annotation toolbar name - Merge remote-tracking branch 'origin/master' into new-annotation-toolbar_ToggleActionMenu -

D15580: [WIP] New annotation toolbar

simgunz edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc: simgunz, ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, andisa, siddharthmanthan, maguirre,

D15580: [WIP] New annotation toolbar

ngraham added a comment. If you don't like the proposed vertical toolbar, that's okay, but then we're going to need to sort out a lot of tricky UI issues for the horizontal toolbar and have perfect icons. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To:

Re: D15580: [WIP] New annotation toolbar

Sorry for the long silence. I have been busy at work and then on vacation. Personally I do not like the idea of the vertical toolbar specifically for Okular for the following reasons: - Okular differently from Gwenview already has a tab bar, which means that further splitting the reviews tab into

D15580: [WIP] New annotation toolbar

simgunz added a comment. Sorry for the long silence. I have been busy at work and then on vacation. Personally I do not like the idea of the vertical toolbar specifically for Okular for the following reasons: - Okular differently from Gwenview already has a tab bar, which means

D15580: [WIP] New annotation toolbar

ngraham added a comment. In D15580#515250 , @davidhurka wrote: > In D15580#514707 , @ngraham wrote: > > > In D15580#513826 , @simgunz wrote: > > > >

D15580: [WIP] New annotation toolbar

davidhurka added a comment. In D15580#514707 , @ngraham wrote: > In D15580#513826 , @simgunz wrote: > > > - How would you fit the annotation actions in the Reviews tab? > > - Would you create a

D15580: [WIP] New annotation toolbar

ngraham added a comment. In D15580#513826 , @simgunz wrote: > - How would you fit the annotation actions in the Reviews tab? > - Would you create a sub-tab in it (as in Gwenview where the tabs are at the bottom)? - > - Can you provide a

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 64017. simgunz added a comment. - Change typewriter and pin icons REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=62679=64017 BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL

D15580: [WIP] New annotation toolbar

simgunz added a comment. In D15580#513253 , @ngraham wrote: > Some thoughts: > > - Instead of using a horizontal toolbar below the main toolbar, instead I might experiment with a vertical toolbar like the one shown in Gwenview's View mode.

D15580: [WIP] New annotation toolbar

simgunz edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc: ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, maguirre, fbampaloukas, joaonetto, kezik, tfella,

D15580: [WIP] New annotation toolbar

ngraham added a comment. In D15580#513395 , @davidhurka wrote: > In D15580#513253 , @ngraham wrote: > > > Some thoughts: > > > These thoughts sound good to me. > > > - Show a button on

D15580: [WIP] New annotation toolbar

davidhurka added a comment. In D15580#513253 , @ngraham wrote: > Some thoughts: These thoughts sound good to me. > - Show a button on the toolbar by default that holds the list of favorite annotations, [...] "Quick annotations". At

D15580: [WIP] New annotation toolbar

ngraham added a comment. Some thoughts: - Instead of using a horizontal toolbar below the main toolbar, instead I might experiment with a vertical toolbar like the one shown in Gwenview's View mode. We could make the toolbar live inside the Reviews tab, which appears to be somewhat

D15580: [WIP] New annotation toolbar

ngraham added a comment. Thanks, I was indeed not seeing the latest version. After deleting the necessary config files, I see the latest version in its own toolbar below the main one. This is a lot better than the current one already, but I think we can polish it even more. I have a lot of

D15580: [WIP] New annotation toolbar

simgunz added a comment. In D15580#506692 , @ngraham wrote: > For my own UI review, I must continue to push for putting the annotation tools on a second toolbar that appears below the main one when needed. The tools are already on a

D15580: [WIP] New annotation toolbar

ngraham added a reviewer: VDG. ngraham added a comment. Thanks @simgunz, this is really excellent work. Adding @VDG for more UI review comments. For my own UI review, I must continue to push for putting the annotation tools on a second toolbar that appears below the main one when

D15580: [WIP] New annotation toolbar

ngraham edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, maguirre, fbampaloukas, joaonetto, kezik, tfella, darcyshen

D15580: [WIP] New annotation toolbar

ngraham edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, maguirre, fbampaloukas, joaonetto, kezik, tfella, darcyshen

D15580: [WIP] New annotation toolbar

simgunz added a comment. With the latest push all the features of the toolbar are implemented. There are some horrible things in the code, but I will clean and refactor it. Now it is time for you to test the toolbar and provide me feedback on the UI / UX please. (No need to comment on

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 62679. simgunz added a comment. - Fix detach annotation when mouse mode unchecked REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=62678=62679 BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 62678. simgunz added a comment. - Use ToggleActionMenu - Add stamp annotation - Fix swap straight line and arrow - Use Title style capitalization - Show real stamp symbols if squared - Better stamp icon REPOSITORY R223 Okular CHANGES SINCE

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 62478. simgunz added a comment. - Add geometrical annotations to action collection - Move arrow before straight line - Allow setting action text - Set default shortcuts - Set favorite tools default shortcuts - Fix i18n argument - Auto-update

D15580: [WIP] New annotation toolbar

simgunz added a comment. > Do you have to move this to gitlab anyway? I do not plan to move it given that all the discussion was done here, and that I have not yet seen an official guide regarding the review requests through gitlab. REPOSITORY R223 Okular REVISION DETAIL

D15580: [WIP] New annotation toolbar

ltoscano added a comment. In D15580#500275 , @davidhurka wrote: > In D15580#500264 , @simgunz wrote: > > > @davidhurka Thanks for looking up the bug numbers > > > > Do you know if adding the

D15580: [WIP] New annotation toolbar

davidhurka added a comment. In D15580#500264 , @simgunz wrote: > @davidhurka Thanks for looking up the bug numbers > > Do you know if adding the bug numbers separated by a comma without repeating the keyword BUG would work? Probably

D15580: [WIP] New annotation toolbar

simgunz added a comment. @davidhurka Thanks for looking up the bug numbers Do you know if adding the bug numbers separated by a comma without repeating the keyword BUG would work? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc:

D15580: [WIP] New annotation toolbar

simgunz edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, maguirre, fbampaloukas, joaonetto, kezik, tfella, darcyshen

D15580: [WIP] New annotation toolbar

simgunz edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, maguirre, fbampaloukas, joaonetto, kezik, tfella, darcyshen

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 62320. simgunz added a comment. - Check if chosen color is valid before storing it - Notify PageViewAnnotator when the color has changed - Add missing annotation tools actions - Add XML annotation tools and connect corresponding actions - Add

D15580: [WIP] New annotation toolbar

davidhurka added a comment. You could consider to add these bugs as BUG keywords to this patch: 386578, 374728, 352310, 330518, 341914, 157289 - https://bugs.kde.org/show_bug.cgi?id=386578 Allow annotation mode to "stick" for better use with a stylus -

D15580: [WIP] New annotation toolbar

simgunz added a comment. I messed up something... I see many files I have not modified that are marked as modified here. Probably because I changed the origin remote and my master was not in sync with upstream. I'll fix this with the next arc diff, now it does not let me arc diff again

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 61742. simgunz added a comment. - Fix action deselected if favorite tool with the same type is activated - Fix geom shapes action disabled when okular reparses config - Disable text annotation when file does not support them REPOSITORY R223 Okular

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 61190. simgunz added a comment. - Add i18n - Move default value to declaration - Add tooltips REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=61188=61190 BRANCH new-annotation-toolbar REVISION DETAIL

D15580: [WIP] New annotation toolbar

simgunz marked 6 inline comments as done. simgunz added a subscriber: cfeck. simgunz added a comment. I have managed to re-enable the action Tools > Review to show/hide the annotation toolbar. This action needs to be defined in shell.cpp, because to use KToggleToolBarAnnotation we need

D15580: [WIP] New annotation toolbar

simgunz updated this revision to Diff 61188. simgunz added a comment. - Clean parseTool - Initialize private members - Add action to toggle annotation toolbar, set default position below REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE

D15580: [WIP] New annotation toolbar

simgunz retitled this revision from "New annotation toolbar" to "[WIP] New annotation toolbar". REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: aacid, anthonyfieroni, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel,