Review Request: Fix "set presence status here" bug

George Kiagiadakis kiagiadakis.george at gmail.com
Thu Sep 29 13:40:40 UTC 2011


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



package/contents/ui/EditableText.qml
<http://git.reviewboard.kde.org/r/102736/#comment6122>

    It took me a while to understand this. I don't like it at all, duplicating logic and mixing 3-4 different properties together is a bad idea. How about moving all the logic in the top level element like this:
    
    onEditableTextChanged: {
       if (editableText == "") {
          shownText.text = editText.clickMessage;
       } else {
          shownText.text = editText.text;
       }
       editText.text = editableText;
       textChanged(editableText);
    }
    
    Then in editText.onReturnPressed you just do:
    
       textEditItem.editableText = text;
    
    and the Component.onCompleted handler here is not needed.



package/contents/ui/EditableText.qml
<http://git.reviewboard.kde.org/r/102736/#comment6121>

    To my understanding, clickMessage is a property that should be set once when the LineEdit is constructed and then never touched again. The LineEdit itself should take care of showing that if its text is empty. So, this could just become:
    
      clickMessage: i18n("Click to set text");
    
    It could also be an external property, so that the caller can set a customized clickMessage...
    
      clickMessage: textEditItem.clickMessage;


- George Kiagiadakis


On Sept. 29, 2011, 12:19 p.m., Francesco Nwokeka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102736/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2011, 12:19 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Fixes the bug stated in the bug report. I've added a "clickMessage" to the lineedit when text is empty.
> 
> 
> This addresses bug 282210.
>     http://bugs.kde.org/show_bug.cgi?id=282210
> 
> 
> Diffs
> -----
> 
>   package/contents/config/main.xml 6cb185a 
>   package/contents/ui/EditableText.qml 23e86df 
>   package/contents/ui/LauncherPanel.qml 999748a 
>   package/contents/ui/main.qml 6739d27 
> 
> Diff: http://git.reviewboard.kde.org/r/102736/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Francesco Nwokeka
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20110929/06969581/attachment.html>


More information about the KDE-Telepathy mailing list