[Kde-graphics-devel] Review Request 123342: ksnapshot: Make "SendTo..." menu smaller (by introducing a "More" submenu) and user-configurable

[Aaron J. Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123342/#review79900
-----------------------------------------------------------


So firstly, including a dependency on knewstuff just to get at the KMoreTools is imho not acceptable. KMoreTools does not belong in KNewStuff. That is the entire point of frameworks: modularity, ability to choose based on specific functionality.

Otherwise, there are a few comments in the code below.


ksnapshotsendtoactions.cpp (lines 50 - 62)
<https://git.reviewboard.kde.org/r/123342/#comment54747>

    This does not belong in the code. It will eventually be outdated and comments in code like this bitrot quickly. Place it in the README perhaps.



ksnapshotsendtoactions.cpp (lines 129 - 132)
<https://git.reviewboard.kde.org/r/123342/#comment54748>

    why not just
    
    if (!moreMenu) {
         moreMenu = menu;
    }
    
    moreMenu->addSeparator();
    [..]
    
    ?



ksnapshotsendtoactions.cpp (lines 200 - 206)
<https://git.reviewboard.kde.org/r/123342/#comment54749>

    you can't rely on ordering anyways, as the user may install new plugins (or remove existing ones) between launches. so you need something identifiable. Does kipi happen to add anything useful to the QAction* like a QVariant in data(), or perhaps even a QObject::objectName()?
    
    That would at least be more stable...


- Aaron J. Seigo


On April 15, 2015, 9:15 p.m., Gregor Mi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123342/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 9:15 p.m.)
> 
> 
> Review request for KDE Graphics, KSnapshot and Aaron J. Seigo.
> 
> 
> Bugs: 274928, 287663 and 319901
>     http://bugs.kde.org/show_bug.cgi?id=274928
>     http://bugs.kde.org/show_bug.cgi?id=287663
>     http://bugs.kde.org/show_bug.cgi?id=319901
> 
> 
> Repository: ksnapshot
> 
> 
> Description
> -------
> 
> Currently, ksnapshot's "Send To..." menu is one long list of items which can be quite large depending on the number of services and kipi-plugins installed. Note, that in the provided screenshot, the list is not that long because not all kipi plugins are ported yet.
> In my daily screenshot work I always use the same set of tools and therefore it would be nice if the menu was user-configurable.
> 
> SUGGESTION:
> - Only show up to 10 items in the main menu and move the rest to a new "More" submenu.
> - Implement with KMoreTools (see https://git.reviewboard.kde.org/r/122910/)
> - Since KMoreTools is used, the menu is automatically user configurable. See screenshots.
> 
> 
> Diffs
> -----
> 
>   ksnapshotsendtoactions.cpp a8c4ccbb72cee3bef486af417aa637c3f41de48d 
>   ksnapshotsendtoactions.h f0b4f8f6ccea15b169d7ef149645e85d89b5fe37 
>   ksnapshot.cpp ac0f2c4f44d47e1a4ae4a318382253a23ad1ed4a 
>   CMakeLists.txt cdab928a7db0e7ea29fba35880250639461dbb1e 
>   CMakeLists.Sources.txt 522fc3f0f943e5c2856c76e93174d39661ca9c50 
> 
> Diff: https://git.reviewboard.kde.org/r/123342/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> current situation - SendTo menu
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/12/a7bf453e-0d86-46b6-a593-11a529fcdb38__current_sendto_menu.png
> Reduced SendTo menu after custom configuration
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/12/b933c1b7-cd71-43ee-a7a0-48fb7a2567ab__1-sendt.png
> remaining items in "More" submenu
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/12/ad5eb6c4-e206-4e06-8735-c8a7f422676e__2-sendto-more.png
> Configure Menu dialog
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/12/8d7a7368-3567-4f2b-b2f7-0a8b619549ec__3-configure-dialog-1.png
> Configure Menu dialog after Reset was clicked (first 10 items in "Main section" by default)
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/12/cc0c6f03-7257-442c-86f5-6f4a7b8e281e__4-configure-dialog-after-reset.png
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-graphics-devel/attachments/20150505/887b77a2/attachment.html>