Review Request 124048: Krita - Brush editor Stamp and Clipboard refactoring [WIP]

[Stefano Bonicatti]

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

Review request for Calligra.


Bugs: 345195
    http://bugs.kde.org/show_bug.cgi?id=345195


Repository: calligra


Description
-------

The Stamp and Clipboard tabs are removed and they become buttons in the Predefined tab, so that the user must add a predefined brush tip to use it first.
This solves some issues and inconsistencies with temporary brush tips by removing the need of them.
    
Fixes a crash that happened when selecting a temporary brush after adding the respective clone to the predefined or when deleting the cloned brush.
The crash happened only with a Stamp Animated one.
    
Fixes a crash when attempting to add a Clipboard brush tip when the clipboard is empty. 
Now the OK button will be disabled if the clipboard is emmpty.

This review is here because of the placing of the buttons, their text and tooltips.
David Revoy suggested to put "long" (but that fit on the button) text on them instead of icons and to write a tooltip to explain what they do.
I wanted to put the buttons on the same line of the import and delete ones but given that they come from another widget and that the row is then occupied, i couldn't.
Another possible issue is that the buttons now open a dialog so the brush editor popup closes in the background and there's "no" way to keep it open or reopen it.
Or well there's a way to reopen it but it involves calling 7 nested parentWidget, and then show().

https://mail.kde.org/pipermail/kimageshop/2015-June/012772.html here a little more in depth explanation on this change.

Now if the popup closing is too anoying, the only way i see is to go back to tabs, but this time the brush won't be usable until add to predefined is clicked (while previously, due to the temporary brush created, just by opening the respective tab it was possible to use it).


Diffs
-----

  krita/libbrush/kis_imagepipe_brush.cpp 4d301cb 
  krita/plugins/paintops/libpaintop/forms/wdgclipboardbrush.ui bb1cd4a0 
  krita/plugins/paintops/libpaintop/forms/wdgcustombrush.ui 043252b 
  krita/plugins/paintops/libpaintop/kis_brush_chooser.h aea4cd3 
  krita/plugins/paintops/libpaintop/kis_brush_chooser.cpp 8f46885 
  krita/plugins/paintops/libpaintop/kis_brush_selection_widget.h 2462917 
  krita/plugins/paintops/libpaintop/kis_brush_selection_widget.cpp 1f61266 
  krita/plugins/paintops/libpaintop/kis_clipboard_brush_widget.h 62f6aea 
  krita/plugins/paintops/libpaintop/kis_clipboard_brush_widget.cpp 58b18e1 
  krita/plugins/paintops/libpaintop/kis_custom_brush_widget.h 13589d0 
  krita/plugins/paintops/libpaintop/kis_custom_brush_widget.cpp 630aff5 

Diff: https://git.reviewboard.kde.org/r/124048/diff/


Testing
-------

Several adding/removing/selecting tests in the predefined list of both Stamp and Clipboard brush.


Thanks,

Stefano Bonicatti

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20150608/79837e07/attachment.htm>