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

[Boudewijn Rempt]

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

Ship it!


Ship It!

- Boudewijn Rempt


On June 30, 2015, 5:38 p.m., Stefano Bonicatti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124048/
> -----------------------------------------------------------
> 
> (Updated June 30, 2015, 5:38 p.m.)
> 
> 
> 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/extensions/dockers/defaultdockers/kis_layer_box.cpp 810ae21 
>   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 
>   libs/widgets/KoResourceItemChooser.h 74b404c 
>   libs/widgets/KoResourceItemChooser.cpp 86e3663 
> 
> 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/20150808/085eea47/attachment.htm>