> 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
> 
>

_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Reply via email to