Review Request 118819: Simplify AssistantPopup related code

Kevin Funk kfunk at kde.org
Thu Jun 19 09:27:25 UTC 2014



> On June 18, 2014, 9:59 p.m., Milian Wolff wrote:
> > shell/assistantpopup.cpp, line 194
> > <https://git.reviewboard.kde.org/r/118819/diff/1/?file=282438#file282438line194>
> >
> >     imo, it should execute on the release event and also check that the modifier to equal AltModifier

If you press Alt+F4 the action is triggered immediately as well, so it is consistent with triggering other shortcuts.

Note that doing in on the release event also introduces some problems: Pressing Alt then Key_1 together results in '1' being inserted into the text and after the release of Alt immediately triggers the first action in the assistant (if it shows up). It's not easy to disambiguate if this was intended by the user or not.


> On June 18, 2014, 9:59 p.m., Milian Wolff wrote:
> > shell/assistantpopup.cpp, line 216
> > <https://git.reviewboard.kde.org/r/118819/diff/1/?file=282438#file282438line216>
> >
> >     aren't we now always handling everything from within Qt, and can thus omit all of the event filtering stuff here?

See comment above.


- Kevin


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


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/20140619/9b612cf3/attachment.html>


More information about the KDevelop-devel mailing list