Review Request 114765: Bug 322016 - Apply button is always enabled in Playlist Layout Editor dialog

[Nilesh Suthar]

> On Jan. 3, 2014, 12:23 p.m., Matěj Laitl wrote:
> > Hi, thanks for the patch. I think there could be a better approach in solving the bug - the one I've outlined in my review of a similar request: https://git.reviewboard.kde.org/r/113057/
> > 
> > It may turn out not possible/worth it, but it is for sure worth investigation.

1. watch every state change in the applicable controls and pressing of the apply button.
2. when a change happens, compare the current visible configuration with the last saved one and call setEnabled( settingsDiffer ); where settingsDiffer is a bool with obvious meaning.

--You mean that every time in any change check the changed setting and set settingsDiffer variable true or false accordingly.also settingsDiffer will be calculated every time when any change would happen.


> On Jan. 3, 2014, 12:23 p.m., Matěj Laitl wrote:
> > src/playlist/layouts/PlaylistLayoutEditDialog.cpp, line 114
> > <https://git.reviewboard.kde.org/r/114765/diff/2/?file=228521#file228521line114>
> >
> >     Please respect Amarok coding style - spaces around arguments - see the HACKING folder in Amarok sources. This applies to other places in your patch.
> >     
> >     Note that existing calls
> >     buttonBox->button(QDialogButtonBox::Apply)
> >     do not conform to Amarok coding style - you may fix that in a separate review request (not in this one as we want style fixes separate from other fixes)

In buttonBox->button(QDialogButtonBox::Apply) do you means spaces around argument which are already there in code?


- Nilesh


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


On Dec. 31, 2013, 5:04 p.m., Nilesh Suthar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114765/
> -----------------------------------------------------------
> 
> (Updated Dec. 31, 2013, 5:04 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Bugs: 322016
>     https://bugs.kde.org/show_bug.cgi?id=322016
> 
> 
> Repository: amarok
> 
> 
> Description
> -------
> 
> Disabled Apply Button on Editor Creation and Close.on any change the apply button is enabled.
> 
> 
> Diffs
> -----
> 
>   src/playlist/layouts/PlaylistLayoutEditDialog.cpp 99aee2a 
> 
> Diff: https://git.reviewboard.kde.org/r/114765/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Nilesh Suthar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20140104/4971fcc6/attachment-0001.html>