Re: Review Request 124344: Replace k3activelabel classes with QLabel in libkopete

> On júl 21, 2015, 7:03 popoludní, Pali Rohár wrote:
> > libkopete/ui/addressbookselectorwidget.cpp, line 143
> > <https://git.reviewboard.kde.org/r/124344/diff/2/?file=386361#file386361line143>
> >
> >     TextEditorInteraction     = TextSelectableByMouse | TextSelectableByKeyboard | TextEditable
> >     
> >     Why it is needed? Have you looked what this lblHeader means and where is used?
> 
> Pali Rohár wrote:
>     I'm reopening this issue because:
>     1) it does not have comment any comment or answers to my questions
>     2) I bet answer to my second question is "No", because it is really not needed (or correct me if I'm wrong!)
> 
> R.Harish  Navnit wrote:
>     I have looked at this label header and Qt::TextEditorInteraction was not required for the same and hence I updated the patch, replacing it with Qt::NoTextInteraction (since I felt there needn't be any text interaction for a header label)
>     https://git.reviewboard.kde.org/r/124344/diff/2-3/

Remove that explicit call for setting text interaction. It will be easier to understand that "we do not care about it" and default value Qt::LinksAccessibleByMouse is also OK.


> On júl 21, 2015, 7:03 popoludní, Pali Rohár wrote:
> > libkopete/ui/metacontactselectorwidget.cpp, line 21
> > <https://git.reviewboard.kde.org/r/124344/diff/2/?file=386363#file386363line21>
> >
> >     Is this change needed? And if yes it is cosmetic, should be in separate commit and should change all includes qclass.h to QClass. Not just only one. Then we will have big mess.
> 
> R.Harish  Navnit wrote:
>     Since this whole patch does some porting to KDE4/Qt4, and deals directly with the QLabel class, I thought it'd be sensible to do this change, keeping in line with the Qt4 documentation. 
>     
>     Revert it, nevertheless and create a separate patch ?
> 
> Pali Rohár wrote:
>     Rewriting qclass.h to QClass is non functional change and should be done in separate patch. And should change every qclass.h.
> 
> R.Harish  Navnit wrote:
>     Yes, I've reverted this one as well, in the updated patch : https://git.reviewboard.kde.org/r/124344/diff/2-3/

In patch 3 I still see diff which change qlabel.h to QLabel.


- Pali


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


On júl 21, 2015, 9:35 popoludní, R.Harish  Navnit wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124344/
> -----------------------------------------------------------
> 
> (Updated júl 21, 2015, 9:35 popoludní)
> 
> 
> Review request for Kopete, Laurent Montel and Pali Rohár.
> 
> 
> Repository: kopete
> 
> 
> Description
> -------
> 
> Use QLabel instead of K3ActiveLabel
> 
> 
> Diffs
> -----
> 
>   libkopete/ui/addressbookselectorwidget.cpp d1686ed855a94ee5a01db8a4ed0c04d573cd5be4 
>   libkopete/ui/addressbookselectorwidget_base.ui de6ec4d24cd10eebfebc11b4fb9839be206552dc 
>   libkopete/ui/metacontactselectorwidget.cpp 6a45cf2ac9f6d904e7f7363d2fb7e7306f0c6c3a 
>   libkopete/ui/metacontactselectorwidget_base.ui 1f69a48c559ab3f002700068ec6a6c200a9242e3 
> 
> Diff: https://git.reviewboard.kde.org/r/124344/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> 
> Thanks,
> 
> R.Harish  Navnit
> 
>

_______________________________________________
kopete-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kopete-devel