Review Request 118819: Simplify AssistantPopup related code

Milian Wolff mail at milianw.de
Wed Jun 18 21:59:57 UTC 2014


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


Just the amount of code you got rid of speaks for itself. But I wonder whether we can drive this even further? Just "drop" my comments if they are invalid, and go ahead committing it if so. In any case, I'd like to see some explanations on why this can't work out though :)

Cheers, and thanks for working on this!


shell/assistantpopup.cpp
<https://git.reviewboard.kde.org/r/118819/#comment42174>

    imo, it should execute on the release event and also check that the modifier to equal AltModifier



shell/assistantpopup.cpp
<https://git.reviewboard.kde.org/r/118819/#comment42175>

    is this still required?



shell/assistantpopup.cpp
<https://git.reviewboard.kde.org/r/118819/#comment42176>

    aren't we now always handling everything from within Qt, and can thus omit all of the event filtering stuff here?


- Milian Wolff


On June 18, 2014, 9:44 p.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118819/
> -----------------------------------------------------------
> 
> (Updated June 18, 2014, 9:44 p.m.)
> 
> 
> Review request for KDevelop and Sven Brauch.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Simplify AssistantPopup related code
> 
> Get rid off most JS logic inside QML, make it a lot more declarative.
> I'm not sure if I've fixed the losing-focus bug, but I cannot reproduce
> it anymore.
> 
> 
> Diffs
> -----
> 
>   shell/assistantpopup.qml 69ccc2f9f833567e70406e04bae040392ae3f961 
>   shell/assistantpopup.cpp 1f3ff012a51dbab6f1d0c75149e3cf712f1ec449 
>   shell/assistantpopup.h 475cc37c5b205d213adec7fb03c71e5a81f05749 
>   shell/AssistantButton.qml ac3d27bb6eee21a99a04005b62a3a3ef5d5c1930 
> 
> Diff: https://git.reviewboard.kde.org/r/118819/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140618/20298345/attachment.html>


More information about the KDevelop-devel mailing list