Review Request 115196: New declarative based assistant

Milian Wolff mail at milianw.de
Fri Jan 24 12:34:34 UTC 2014


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


please test whether shortcuts which include alt still work - i.e. try to assign "alt + x" to something and see whether that still does something when the assistant is visible

also, please add more documentation to the qml files


shell/assistantpopup.h
<https://git.reviewboard.kde.org/r/115196/#comment34088>

    put this under the Q_OBJECT macro



shell/assistantpopup.h
<https://git.reviewboard.kde.org/r/115196/#comment34089>

    here and below: spurious semicolon



shell/assistantpopup.h
<https://git.reviewboard.kde.org/r/115196/#comment34090>

    const.remove get prefix



shell/assistantpopup.h
<https://git.reviewboard.kde.org/r/115196/#comment34091>

    this is unused - remove and make the name a constant property



shell/assistantpopup.h
<https://git.reviewboard.kde.org/r/115196/#comment34098>

    remove get prefix from getters



shell/assistantpopup.cpp
<https://git.reviewboard.kde.org/r/115196/#comment34093>

    cmath



shell/assistantpopup.cpp
<https://git.reviewboard.kde.org/r/115196/#comment34092>

    use uppercase include



shell/assistantpopup.cpp
<https://git.reviewboard.kde.org/r/115196/#comment34094>

    QVariant::fromValue<QObject*>(m_config)



shell/assistantpopup.cpp
<https://git.reviewboard.kde.org/r/115196/#comment34097>

    this should only be done if its visible, no? esp. considering that this sets the focus on the editor, we'd end up with "esc focuses editor" in a completely unrelated piece of code - ensure that does not happen



shell/uicontroller.cpp
<https://git.reviewboard.kde.org/r/115196/#comment34096>

    what is this being used for? comment!



shell/uicontroller.cpp
<https://git.reviewboard.kde.org/r/115196/#comment34095>

    add comment that this is supposed t reduce flickering


- Milian Wolff


On Jan. 24, 2014, 12:05 a.m., Sven Brauch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115196/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2014, 12:05 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> A new proposal for what the assistant should look like. See screenshots.
> Ideas welcome :)
> 
> 
> Diffs
> -----
> 
>   shell/AssistantButton.qml PRE-CREATION 
>   shell/CMakeLists.txt f840acd 
>   shell/assistantpopup.h 33a462a 
>   shell/assistantpopup.cpp c19a149 
>   shell/assistantpopup.qml PRE-CREATION 
>   shell/uicontroller.cpp fbce314 
>   util/CMakeLists.txt cf2fc1a 
>   util/richtexttoolbutton.h 8d0b1e2 
>   util/richtexttoolbutton.cpp d4263a6 
> 
> Diff: https://git.reviewboard.kde.org/r/115196/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> new assistant, bright colour scheme
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/01/21/96c23c0e-106e-47e0-8f80-90303029e521__assistant_bright.png
> dark colour scheme
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/01/21/d52643d2-3e0d-494b-bdd7-7beac3f4656c__assistant_dark.png
> alt pressed
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/01/21/1df94b0b-d763-4aef-9182-ab113fcc3b1a__assistant_hl.png
> assistant, after triggering an action
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/01/21/c118a296-778c-4cff-86a9-1ff63c2a3e48__assistant_trig.png
> version 2,  bright
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/01/22/80396242-9477-47d0-9b80-238f104ac248__assistant_bright2.png
> version 2, dark
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/01/22/8c531d60-c6ab-45e5-a05c-6639f4ed8225__assistant_dark.png
> 
> 
> Thanks,
> 
> Sven Brauch
> 
>

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


More information about the KDevelop-devel mailing list