Review Request: Request to review this patch submit which fixes the bug #152182

[Anne-Marie Mahfouf]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104138/#review11077
-----------------------------------------------------------


I am not satisfied with the patch.

1) In the View class we already have 2 methods [void sliderWindowClosed(); and void slidersWindowClosed();] for setting the m_menuSliderAction to false. So instead of adding a new method this should be fixed here. And see why there are 2 such methods.
2) Is there a need for a toggle action as the dialog is either shown or not (doesn't it behave exactly as the Coordinate System dialog?) I would make it a simple action "Show Sliders..." to open the dialog.
3) As I said in the mail when we discussed this bug, the sliders behavior should be clarified. According to http://userbase.kde.org/KmPlot/Using_Sliders the sliders modify a parameter value. If there is no parameter, the dialog does nothing, as does nothing checking "Slider 1" in the tab. So should this be available at all in those cases?

I'll bring these points to the mailing list in order to discuss it here.

- Anne-Marie Mahfouf


On March 1, 2012, 10:23 p.m., Rahul Sharma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104138/
> -----------------------------------------------------------
> 
> (Updated March 1, 2012, 10:23 p.m.)
> 
> 
> Review request for KDE Edu, Burkhard Lück and Klaus-Dieter Möller.
> 
> 
> Description
> -------
> 
> Attached two .diff files (patches) which fixes the bug.
> 
> 
> This addresses bug 152182.
>     http://bugs.kde.org/show_bug.cgi?id=152182
> 
> 
> Diffs
> -----
> 
> 
> Diff: http://git.reviewboard.kde.org/r/104138/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

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