D15580: New annotation toolbar

2018-09-17 Thread Simone Gaiarin
simgunz created this revision. simgunz added a reviewer: Okular. Herald added a project: Okular. Herald added a subscriber: okular-devel. simgunz requested review of this revision. REVISION SUMMARY As discussed in the task T8076 I am proposing a new annotatio

D15580: New annotation toolbar

2018-09-17 Thread Simone Gaiarin
simgunz added a task: T8076: Fix design of annotation toolbar. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: okular-devel, ngraham, aacid

D15580: New annotation toolbar

2018-09-17 Thread Simone Gaiarin
simgunz added a comment. To test this code temporary move `.config/okularpartrc` or the annotation tools may not match the ones in the buttons. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: okular-devel, ngraham, aacid

D15580: New annotation toolbar

2018-09-17 Thread Tobias Deiminger
tobiasdeiminger added a comment. Nice approach👍 D1204 , D1205 and D15580 conflict somewhat. Not bad, but whoever gets in later will have to adapt. Shall we try to coordinate? REPOSITO

D15580: New annotation toolbar

2018-09-17 Thread Tobias Deiminger
tobiasdeiminger added a comment. How would the new color picker handle "multi-color" annotations? I ask, because D15205 brings selectable font color for typewriter tool. Inline note is very similar, so I had intended to submit a font color patch for inlin

D15580: New annotation toolbar

2018-09-17 Thread Tobias Deiminger
tobiasdeiminger added a comment. It took some minutes until I realized how to change from ellipse to polygon (long left press). Now I find it good and easy. Would there be a way to make it even more self explaining? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D155

D15580: New annotation toolbar

2018-09-17 Thread Simone Gaiarin
simgunz added a comment. @tobiasdeiminger I'll continue the discussion on the task T8076 related to this diff. So we can discuss the more UI related topics there. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz,

D15580: New annotation toolbar

2018-09-30 Thread Nathaniel Graham
ngraham added a comment. I can't get this to compile on git master: In file included from /home/dev/repos/okular/ui/pageviewannotator.cpp:1245:0: /home/dev/repos/okular/moc_pageviewannotator.cpp: In static member function ‘static void PageViewAnnotator::qt_static_metacall(QObject*,

D15580: New annotation toolbar

2018-10-03 Thread Simone Gaiarin
simgunz updated this revision to Diff 42831. 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 REPOSITORY

D15580: New annotation toolbar

2018-10-03 Thread Simone Gaiarin
simgunz added a comment. Should compile now. Typewriter tool is not included yet. I add few more actions but the color picker works only for text annotation tools for now. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: ngraham, tobiasd

D15580: New annotation toolbar

2018-10-07 Thread Simone Gaiarin
simgunz edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: ngraham, tobiasdeiminger, okular-devel, aacid

D15580: New annotation toolbar

2018-10-07 Thread Simone Gaiarin
simgunz edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: ngraham, tobiasdeiminger, okular-devel, aacid

D15580: New annotation toolbar

2018-10-08 Thread Nathaniel Graham
ngraham added a comment. > **To test this revision** > Temporary move the file .config/okularpartrc or the annotation tools may not match the ones in the buttons. This won't be a necessity for the final version, right? We wouldn't want users to have to do this. REPOSITORY R223 Okul

D15580: New annotation toolbar

2018-10-09 Thread Simone Gaiarin
simgunz added a comment. In D15580#339425 , @ngraham wrote: > > **To test this revision** > > Temporary move the file .config/okularpartrc or the annotation tools may not match the ones in the buttons. > > This won't be a necessity for the

D15580: New annotation toolbar

2018-10-10 Thread Nathaniel Graham
ngraham added a comment. In D15580#339642 , @simgunz wrote: > In D15580#339425 , @ngraham wrote: > > > > **To test this revision** > > > Temporary move the file .config/okularpartrc or the annotat

D15580: New annotation toolbar

2019-05-25 Thread Rajeesh K Nambiar
knambiar added a comment. This change doesn't cleanly apply on the current master branch. Would you rebase the changes, or is there a branch it could be built against (I don't see annotation-toolbar branch yet)? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580

D15580: New annotation toolbar

2019-05-26 Thread Simone Gaiarin
simgunz added a comment. I am going to rebase it to master and push some new changes in a few days. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: knambiar, ngraham, tobiasdeiminger, okular-devel, joaonetto, tfella, darcyshen, aacid

D15580: New annotation toolbar

2019-06-02 Thread Simone Gaiarin
simgunz added a task: T11014: Dimension line annotation tool. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: knambiar, ngraham, tobiasdeiminger, okular-devel, joaonetto, tfella, darcyshen, aacid

D15580: New annotation toolbar

2019-06-02 Thread Simone Gaiarin
simgunz updated this revision to Diff 59017. simgunz added a comment. Still very rough, but most of the tools and corresponding basic settings are now there. Remember to move/remove ~/.config/okularpartrc before testing this. Changes highlights: - Check if chosen color is valid

D15580: New annotation toolbar

2019-06-02 Thread Nathaniel Graham
ngraham added a comment. getting there! UI review of the latest version: - Having the annotation tools on the main toolbar makes it overflow for all but the largest window sizes. How about putting them in a secondary toolbar below the main one that's hidden until the user shows it b

D15580: New annotation toolbar

2019-06-03 Thread Simone Gaiarin
simgunz edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: knambiar, ngraham, tobiasdeiminger, okular-devel, joaonetto, tfella, darcyshen, aacid

D15580: New annotation toolbar

2019-06-03 Thread Simone Gaiarin
simgunz added a comment. There is a TODO list of the working and planned feature on task T8076 . Some of your suggestions are already there and I added the missing one. I have updated the description of this review to point to that TODO for better clarity.

D15580: New annotation toolbar

2019-06-03 Thread Nathaniel Graham
ngraham added a comment. >> - The Color button should actually show the current color rather than a generic icon > > This should already work. My mistake, it totally does work! >> - The Inner color button needs something to show that it exists; right now it just looks like whit

D15580: New annotation toolbar

2019-06-03 Thread David Hurka
davidhurka added a comment. In D15580#473733 , @ngraham wrote: > >> - I can't figure out what Pin Annotation actually does > > > > If checked the current annotation tool is kept selected after use (as double-click does in the current Okular)

D15580: New annotation toolbar

2019-06-03 Thread Anthony Fieroni
anthonyfieroni added inline comments. INLINE COMMENTS > part.rc:2 > > > Increase version. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: anthonyfieroni, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, joaonetto, tfel

D15580: New annotation toolbar

2019-06-03 Thread Nathaniel Graham
ngraham added a comment. In D15580#473781 , @davidhurka wrote: > Using double-click is probably used to some [how many?] Okular users, but others asked for something like a sticky button. An option to make selected actions sticky by default, wi

D15580: New annotation toolbar

2019-06-03 Thread Simone Gaiarin
simgunz added a comment. If we remove the sticky button, we have two options: - set sticky by default, we will make some users that use the non-sticky mode unhappy - set non-sticky by default and use double click to stick, we go back to the current situation (bug 358057) I we keep

D15580: New annotation toolbar

2019-06-04 Thread David Hurka
davidhurka added a comment. In D15580#473824 , @ngraham wrote: > Sticky-by-default would probably be okay as long as we can make it //very// clear how to un-select the tool. Probably implementing multiple methods would be good (hit esc key, left

D15580: New annotation toolbar

2019-06-05 Thread Simone Gaiarin
simgunz added a comment. > Or: Why is this still PageViewToolBar? It is not anymore in the PageView? I'll move it, added to TODO REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular Cc: anthonyfieroni, davidhurka, knambiar, ngraham, tobiasde

D15580: New annotation toolbar

2019-06-05 Thread Simone Gaiarin
simgunz added a comment. In D15580#474436 , @simgunz wrote: > > Or: Why is this still PageViewToolBar? It is not anymore in the PageView? > > I'll move it, added to TODO Actually the code of the other toolbar (Browse, Zoom, Selection)

D15580: New annotation toolbar

2019-06-05 Thread David Hurka
davidhurka added a comment. In D15580#474457 , @simgunz wrote: > In D15580#474436 , @simgunz wrote: > > > > Or: Why is this still PageViewToolBar? It is not anymore in the PageView? > > > > I'll

D15580: New annotation toolbar

2019-06-05 Thread Simone Gaiarin
simgunz updated this revision to Diff 59208. simgunz added a comment. - Use draw-polyline icon for Polygon - Increase version of part.rc - Change icon of strikethrough (remove symbolic suffix) - Add TODO in code - Rename PageViewToolBar to AnnotationActionHandler - No need to forceHid

D15580: New annotation toolbar

2019-06-05 Thread Simone Gaiarin
simgunz added a comment. I renamed PageViewToolBar and moved to a file by itself. I took inspiration from Dolphin for the name. There is still code to be cleaned though. INLINE COMMENTS > pageview.cpp:665 > > -d->aToggleAnnotator = new KToggleAction(QIcon::fromTheme( > QStringLi

D15580: New annotation toolbar

2019-06-11 Thread Albert Astals Cid
aacid added inline comments. INLINE COMMENTS > simgunz wrote in pageview.cpp:665 > Anyone knows how KToggleToolBarAction is supposed to work? > > Currently it does nothing. > > Reading the code, it seems that the constructor where the toolbar is > specified by name does nothing, unless there i

D15580: New annotation toolbar

2019-06-11 Thread Simone Gaiarin
simgunz added inline comments. INLINE COMMENTS > aacid wrote in pageview.cpp:665 > You're right, it seems it never has. Can you use the other one? Is it there a clean way to obtain a reference to the toolbar from within this class? I guess that KMainWindow::toolBar(name) is a possibility, but h

D15580: New annotation toolbar

2019-06-13 Thread Albert Astals Cid
aacid added inline comments. INLINE COMMENTS > annotationactionhandler.cpp:131 > +// Text markup > +QAction * annHighlighter = new QAction(QIcon::fromTheme( > QStringLiteral("draw-eraser") ), "Highlighter", this); > +annHighlighter->setCheckable( true ); no i18n? > simgunz wrote i

D15580: New annotation toolbar

2019-06-13 Thread Simone Gaiarin
simgunz added inline comments. INLINE COMMENTS > aacid wrote in pageview.cpp:665 > right, that's actually a tricky question. > > I'll try to figure it out, but you may want to play with stuff and not wait > for me, a close friend of mine is getting married so don't expect anything > from me un

D15580: New annotation toolbar

2019-06-14 Thread Simone Gaiarin
simgunz updated this revision to Diff 59781. simgunz added a comment. Changes: - Added opacity action - Cleaned width action - Big code refactor: - m_toolDefinition is now a QDomDocument, so that the toolElement are not randomly deleted when the associated QDomDocument goes out of

D15580: New annotation toolbar

2019-06-16 Thread David Hurka
davidhurka added inline comments. INLINE COMMENTS > toolaction.cpp:71 > } > -button->setToolTip( i18n("Click to use the current selection > tool\nClick and hold to choose another selection tool") ); > } Why remove this tooltip? To make ToolAction (or my inferior ToggleAct

D15580: New annotation toolbar

2019-06-16 Thread Simone Gaiarin
simgunz added inline comments. INLINE COMMENTS > davidhurka wrote in toolaction.cpp:71 > Why remove this tooltip? > > To make ToolAction (or my inferior ToggleActionMenu) more universal to use it > for a Change Colors feature, it would be nice if tooltips are not needed > anymore. That toolti

D15580: New annotation toolbar

2019-06-16 Thread Simone Gaiarin
simgunz updated this revision to Diff 59954. simgunz added a comment. Added the favorite tools. Now only the stamp tool is missing. PS: As usual, the code is still messy, I'll refactor it. For now I am more interested in feedback on the UI/UX Commits: - Rename XML actions

D15580: New annotation toolbar

2019-06-16 Thread David Hurka
davidhurka added a comment. In D15580#480855 , @simgunz wrote: > For now I am more interested in feedback on the UI/UX Do you have screenshots? INLINE COMMENTS > simgunz wrote in toolaction.cpp:71 > That tooltip does not appear anywhere

D15580: New annotation toolbar

2019-06-17 Thread Simone Gaiarin
simgunz added a comment. Favorite annotations: The Star adds the currently selected tool to the favorite list, the bookmark symbol displays the list of the favorite tools: F6902170: Screenshot_20190617_165440.png Customize the favorite tools:

D15580: New annotation toolbar

2019-06-17 Thread Simone Gaiarin
simgunz updated this revision to Diff 60004. simgunz added a comment. - Fix and simplify width action - Fix and simplify opacity action - Fix annotation tools actions - Rename color action - Formatting REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1

D15580: New annotation toolbar

2019-06-17 Thread David Hurka
davidhurka added a comment. Don’t have UI feedback that asks for action already in this patch. :) In D15580#481270 , @simgunz wrote: > F6902170: Screenshot_20190617_165440.png - Maybe the

D15580: New annotation toolbar

2019-06-17 Thread Simone Gaiarin
simgunz added a comment. In D15580#481323 , @davidhurka wrote: > Don’t have UI feedback that asks for action already in this patch. :) > > In D15580#481270 , @simgunz wrote: > > > F6902170: Scre

D15580: New annotation toolbar

2019-06-20 Thread Simone Gaiarin
simgunz updated this revision to Diff 60168. simgunz added a comment. - Remove unuseful annotation separators - Correctly set favorites action enabled - Properly trigger favorite action - Allow unchecking annotation actions - Minor refactor - Save state of continuous mode REPOSITORY

D15580: New annotation toolbar

2019-10-27 Thread Simone Gaiarin
simgunz retitled this revision from "[WIP] New annotation toolbar" to "New annotation toolbar". REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc: trickyricky26, simgunz, ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeim

D15580: New annotation toolbar

2019-10-27 Thread TrickyRicky
trickyricky26 added a comment. In D15580#554598 , @simgunz wrote: > Is the icon fine for the toolbar hide/close button? Do you have a better suggestion? > > F7674605: Screenshot_20191027_092856.png >

D15580: New annotation toolbar

2019-10-27 Thread Simone Gaiarin
simgunz added a comment. > Semantically, the icon you used (something like `tab-close`) would be correct, but I think it looks too much like a destructive action, with it being red in a circle. Considering this only closes the toolbar, which you can open again at any time, I think this icon

D15580: New annotation toolbar

2019-10-27 Thread Nathaniel Graham
ngraham added a comment. `window-close-symbolic` is black instead of red FWIW. Maybe we should just use that for now @trickyricky26 and #vdg , I do agree that our red close buttons draw too much attention to themselves for non-destructive close act

D15580: New annotation toolbar

2019-10-27 Thread Yuri Chornoivan
yurchor added a comment. Can the patch be rebased against the current git/master for testing and preliminary documentation rewrite? Thanks in advance for your work. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc: yurchor, tri

D15580: New annotation toolbar

2019-10-27 Thread Simone Gaiarin
simgunz updated this revision to Diff 68843. simgunz added a comment. - Merge remote-tracking branch 'origin/master' into new-annotation-toolbar_ToggleActionMenu REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=68813&id=68843 BRANCH new-annotation

D15580: New annotation toolbar

2019-10-27 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. I'm happy with the UI now. :) Subsequent improvements and polish can be delivered in follow-up patches. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISIO

D15580: New annotation toolbar

2019-10-27 Thread Albert Astals Cid
aacid added a comment. Doesn't need this a docbook update? REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: yurchor, trickyricky26, simgunz, ltoscano, cfeck, aacid, davidhur

D15580: New annotation toolbar

2019-10-28 Thread Luigi Toscano
ltoscano added a comment. I agree, please don't commit such a big change without updating the documentation at the same time. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham C

D15580: New annotation toolbar

2019-10-28 Thread Simone Gaiarin
simgunz added a comment. In D15580#554972 , @aacid wrote: > Doesn't need this a docbook update? I think that @yurchor is taking care of the documentation. We also need to complete the other reviews on which this one depends, so I am

D15580: New annotation toolbar

2019-10-31 Thread Andreas Hurka
andreashurka added a comment. I've just tested this on manjaro-kde Operating System: Manjaro Linux KDE Plasma Version: 5.17.1 KDE Frameworks Version: 5.63.0 Qt Version: 5.13.1 git clone ... git apply D15580.diff mkdir build && cd build cmake .. make sudo m

D15580: New annotation toolbar

2019-10-31 Thread Nathaniel Graham
ngraham added a comment. In D15580#557314 , @andreashurka wrote: > - stamps won't get printed (both to pdf and paper) This is a pre-existing issue: https://bugs.kde.org/show_bug.cgi?id=383651 Definitely needs to be fixed eventually o

D15580: New annotation toolbar

2019-10-31 Thread Simone Gaiarin
simgunz added a comment. Thanks for testing this. In D15580#557314 , @andreashurka wrote: > - when setting the opacity to something below 100% through the toolbar button Opacity it gets set to 0% I cannot reproduce this. Does this h

D15580: New annotation toolbar

2019-10-31 Thread Andreas Hurka
andreashurka added a comment. In D15580#557433 , @simgunz wrote: > I cannot reproduce this. Does this happen for all the annotations? Can you set the opacity of a tool to 90%, close Okular and then post the content of `./config/okularpartrc`? Ha

D15580: New annotation toolbar

2019-11-01 Thread Andreas Hurka
andreashurka added a comment. In D15580#557463 , @andreashurka wrote: > In D15580#557433 , @simgunz wrote: > > > I cannot reproduce this. Does this happen for all the annotations? Can you set the o

D15580: New annotation toolbar

2019-11-02 Thread Yuri Chornoivan
yurchor added inline comments. INLINE COMMENTS > annotationactionhandler.cpp:387 > +QAction * aCustomColor = new QAction( QIcon::fromTheme( > QStringLiteral("color-picker") ), > + i18nc("@item:inlistbox", "Custom > Color.."), q ); > +aColorPicker

D15580: New annotation toolbar

2019-11-02 Thread David Hurka
davidhurka added a comment. I managed to test your patch now. Nice to see the ToggleActionMenu working, this is what I had in mind once. Now I see that it can behave somehow weird: When e. g. Ellipsis is selected, and you select again Ellipsis from the Geometric Shape button menu, it get

D15580: New annotation toolbar

2019-11-03 Thread Simone Gaiarin
simgunz added a comment. > Correct me if I'm wrong, but the current version (repos) doesn't have the annotation toolbar so I cannot use it to tweak settings. > Or am I missing something? Right. You have to go through the settings to change anything in the current version. > Chang

D15580: New annotation toolbar

2019-11-03 Thread Simone Gaiarin
simgunz added inline comments. INLINE COMMENTS > davidhurka wrote in shell.cpp:370 > This (maybe) causes one problem now. The default is to open new documents in > new windows. > > Open a new document with the Open action of an existing shell, so a new > Okular window appears. Open the annotat

D15580: New annotation toolbar

2019-11-03 Thread Andreas Hurka
andreashurka added a comment. In D15580#558239 , @simgunz wrote: > Ok. So it is strange that the opacity does not work when you change it through the annotation toolbar. You should check/post your `~/.config/okularpartrc` because there you can s

D15580: New annotation toolbar

2019-11-04 Thread Simone Gaiarin
simgunz added a comment. > I did that for a few minutes now and figured out that the same happens with line width (arrow) as well, as long as you choose x.5 . Looks like the toolbar menus are saving double/float/long (whatever) in a different format from how the menu does: > Toolbar ="0\

D15580: New annotation toolbar

2019-11-04 Thread Simone Gaiarin
simgunz updated this revision to Diff 69254. simgunz added a comment. - Use full ellipsis - Ignore locale when saving numbers in config files REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=68843&id=69254 BRANCH new-annotation-toolbar_ToggleActi

D15580: New annotation toolbar

2019-11-04 Thread David Hurka
davidhurka added inline comments. INLINE COMMENTS > simgunz wrote in shell.cpp:370 > Good that you spotted this bug, though it is unrelated to this change (I > think). If you use Tools > Annotations or the shortcut `F6` everything is > working correctly. The problem is only related to the Hide

D15580: New annotation toolbar

2019-11-05 Thread Nathaniel Graham
ngraham edited the summary of this revision. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: andreashurka, yurchor, trickyricky26, simgunz, ltoscano, cfeck, aacid, davidhurka,

D15580: New annotation toolbar

2019-11-07 Thread Simone Gaiarin
simgunz updated this revision to Diff 69379. simgunz added a comment. Herald added a project: Documentation. Herald added a subscriber: kde-doc-english. - Add setToolBarVisible method - Fix annotation toolbar only hidden in first main window REPOSITORY R223 Okular CHANGES SINCE LAST UPDAT

D15580: New annotation toolbar

2019-11-07 Thread Simone Gaiarin
simgunz updated this revision to Diff 69380. simgunz added a comment. - Merge remote-tracking branch 'origin/master' into new-annotation-toolbar_ToggleActionMenu REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=69379&id=69380 BRANCH new-annotation

D15580: New annotation toolbar

2019-11-07 Thread Simone Gaiarin
simgunz marked 2 inline comments as done. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: kde-doc-english, andreashurka, yurchor, trickyricky26, simgunz, ltoscano, cfeck, aaci

D15580: New annotation toolbar

2019-11-07 Thread Simone Gaiarin
simgunz added a comment. > Might it be a hint that there are too many toolbars? I think only the part should have the annotation toolbar, so it is clear which is affected by the hide action. Not defining the annotation toolbar also in `shell.rc` makes the implementation of the action to

D15580: New annotation toolbar

2019-11-07 Thread Simone Gaiarin
simgunz updated this revision to Diff 69405. simgunz added a comment. - Minor refactor REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=69380&id=69405 BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15

D15580: New annotation toolbar

2019-11-07 Thread Simone Gaiarin
simgunz updated this revision to Diff 69406. simgunz added a comment. - Apply KDE Frameworks astyle REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15580?vs=69405&id=69406 BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricato

D15580: New annotation toolbar

2019-11-07 Thread Nathaniel Graham
ngraham added a comment. Still looks great to me in testing. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: kde-doc-english, andreashurka, yurchor, trickyricky26, simgunz,

D15580: New annotation toolbar

2019-11-10 Thread Nathaniel Graham
ngraham added a dependent revision: D25229: Update docs to the new annotation toolbar. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: kde-doc-english, andreashurka, yurchor,

D15580: New annotation toolbar

2020-02-03 Thread Nathaniel Graham
ngraham added a comment. Looks like the dependent patch has landed, so we can more forward with this. Still LGTM. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: kde-d

D15580: New annotation toolbar

2020-02-03 Thread Simone Gaiarin
simgunz added a comment. aacid requested to write some autotest for the ToggleActionMenu before merging this. I'll merge master in this review and work on the autotests soon. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.

D15580: New annotation toolbar

2020-02-03 Thread Nathaniel Graham
ngraham added a comment. Thanks! REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: kde-doc-english, andreashurka, yurchor, trickyricky26, simgunz, ltoscano, cfeck, aacid, da

D15580: New annotation toolbar

2020-02-03 Thread Albert Astals Cid
aacid added a comment. This is huge, i would really welcome if it could be moved over to invent.kde.org since i'm 92% sure some of the clazy/clang-tidy checks will fail. REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D

D15580: New annotation toolbar

2020-02-04 Thread David Hurka
davidhurka added a comment. In D15580#605611 , @simgunz wrote: > aacid requested to write some autotest for the ToggleActionMenu before merging this. I'll merge master in this review and work on the autotests soon. It’s okay to me that yo

D15580: New annotation toolbar

2020-02-04 Thread Simone Gaiarin
simgunz added a comment. In D15580#605913 , @davidhurka wrote: > In D15580#605611 , @simgunz wrote: > > > aacid requested to write some autotest for the ToggleActionMenu before merging this. I'll m

D15580: New annotation toolbar

2020-02-04 Thread Albert Astals Cid
aacid added a comment. In D15580#605913 , @davidhurka wrote: > In D15580#605611 , @simgunz wrote: > > > aacid requested to write some autotest for the ToggleActionMenu before merging this. I'll mer

D15580: New annotation toolbar

2020-02-04 Thread Yuri Chornoivan
yurchor added a comment. It would be nice if someone can test compile and run Kile with the new toolbar. Kile cannot be stared (immediately crashes) here. I have Kile and Okular from git/master. However, Kile works fine with Okular from release/19.12 branch. Thanks in advance f

D15580: New annotation toolbar

2020-02-04 Thread Albert Astals Cid
aacid added a comment. In D15580#606046 , @yurchor wrote: > It would be nice if someone can test compile and run Kile with the new toolbar. > > Kile cannot be stared (immediately crashes) here. I have Kile and Okular from git/master. > >

D15580: New annotation toolbar

2020-02-04 Thread Albert Astals Cid
aacid added a comment. @simgunz now that we have https://invent.kde.org/kde/okular/merge_requests/105 can you discard this one? REPOSITORY R223 Okular BRANCH new-annotation-toolbar_ToggleActionMenu REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham

D15580: New annotation toolbar

2020-02-06 Thread Simone Gaiarin
simgunz abandoned this revision. simgunz added a comment. Moved to KDE invent. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: kde-doc-english, andreashurka, yurchor, trickyricky26, simgunz, ltoscano, cfeck, aacid, davidh

D15580: New annotation toolbar

2020-02-22 Thread David Hurka
davidhurka added a comment. In D15580#606022 , @aacid wrote: > In D15580#605913 , @davidhurka wrote: > > > In D15580#605611 , @simgunz wrote: > > > > >

D15580: New annotation toolbar

2020-02-22 Thread Yuri Chornoivan
yurchor added a comment. > And someone needs to explain how to create merge requests on invent.kde.org. I tried three ways and failed, and unfortunately I can only find instructions for Phabricator. 1. Fork the repo with the "Fork" button here: https://invent.kde.org/kde/okular

D15580: New annotation toolbar

2020-02-22 Thread Simone Gaiarin
simgunz added a comment. When you create the merge request: - set the title to `WIP: title of merge request` to mark it as work in progress - push the commits as you work so you have a backup, can see the results of the CI tests and possibly receive feedbacks REPOSITORY R223 Okular

D15580: New annotation toolbar

2020-02-22 Thread Nathaniel Graham
ngraham added a comment. See also https://community.kde.org/Infrastructure/GitLab REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg, ngraham Cc: kde-doc-english, andreashurka, yurchor, trickyricky26, simgunz, ltoscano, cfeck, aacid, dav