Review Request 116788: Added ExpandAll() and CollapseAll() KActions.

Amarvir Singh amarvir.ammu.93 at gmail.com
Fri Mar 14 02:17:11 UTC 2014


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


Good work, btw. Thanks for the patch. :) Here are a few points about it:

1. There isn't really a need to expand/collapse WordTypeView when the KAction is called. The user isn't expecting it and didn't call for it.
2. As the only work to be performed by the KAction is to call the LessonView's own methods (expandAll/collapseAll), there isn't a need for a signal to be emitted and then performing stuff in the Editor. This can all be done in LessonView itself. Let's keep things clean and tidy, this helps other code readers as well. :)


src/editor/editor.cpp
<https://git.reviewboard.kde.org/r/116788/#comment37248>

    Don't think WordTypeView's state should be changed.



src/vocabulary/lessonview.cpp
<https://git.reviewboard.kde.org/r/116788/#comment37247>

    Petty issue: Add an extra new line in between the methods.


- Amarvir Singh


On March 14, 2014, 1:41 a.m., Swarn Kumar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116788/
> -----------------------------------------------------------
> 
> (Updated March 14, 2014, 1:41 a.m.)
> 
> 
> Review request for KDE Edu.
> 
> 
> Bugs: -, 236177 and Bug
>     http://bugs.kde.org/show_bug.cgi?id=-
>     http://bugs.kde.org/show_bug.cgi?id=236177
>     http://bugs.kde.org/show_bug.cgi?id=Bug
> 
> 
> Repository: parley
> 
> 
> Description
> -------
> 
> Added ExpandAll() and CollapseAll() KActions, connected them to their corresponding slots. 
> 
> 
> Diffs
> -----
> 
>   src/editor/editor.h e24a99c 
>   src/editor/editor.cpp 334e211 
>   src/vocabulary/lessonview.h fce03fc 
>   src/vocabulary/lessonview.cpp 13c4d5e 
> 
> Diff: https://git.reviewboard.kde.org/r/116788/diff/
> 
> 
> Testing
> -------
> 
> Done :D
> 
> 
> Thanks,
> 
> Swarn Kumar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20140314/b4aef042/attachment.html>


More information about the kde-edu mailing list