Review Request 112231: Remove the area switching tabs

Sven Brauch svenbrauch at gmx.de
Fri Aug 23 23:38:13 UTC 2013


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


Good change! Just a few comments:

What I do not like is that we have buttons which have a dropdown menu with only one entry, and that twice (if we, as discussed (?) merge the "cancel jobs" action in Debug into "back to code"). I'd suggest to have a non-clickable label saying e.g. "<Icon> Review" and a button next to it titled "Back to code", and the menu only for the Code tab.

Then, sometimes the arrow for "Code" is still inside the text ...

And I think "Back to code" in Review must cancel the review mode. It's totally confusing to have this mode enabled outside of the Review area, since you have no idea how to turn it off. With this patch it becomes even less obvious how to turn it off.


shell/areadisplay.cpp
<http://git.reviewboard.kde.org/r/112231/#comment28463>

    Can you put a comment about what this does? I have no idea.



shell/uicontroller.cpp
<http://git.reviewboard.kde.org/r/112231/#comment28464>

    Empty method, do we need to keep it?



sublime/mainwindow.cpp
<http://git.reviewboard.kde.org/r/112231/#comment28465>

    Another empty method?


- Sven Brauch


On Aug. 23, 2013, 11:06 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112231/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2013, 11:06 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> The area switching tabs is something that has bothered me quite a bit recently. It's something that is always visible in the screen and we barely use it. There's very little point to explicitly changing to an area, we usually do it from an action: debug, show differences, etc. These are specified by a new Area::addAction(QAction*) method.
> 
> This patch changes the current tab interface (inspired from Eclipse IIRC), for a button that tells the user what's the current area and where we can go.
> 
> The patch also removes the tabs and some unneeded abstractions in sublime/mainwindow that where only used by the tabs.
> 
> 
> Diffs
> -----
> 
>   shell/CMakeLists.txt fe5cd9b 
>   shell/areadisplay.h PRE-CREATION 
>   shell/areadisplay.cpp PRE-CREATION 
>   shell/mainwindow.h 2050219 
>   shell/mainwindow.cpp d4f4bcb 
>   shell/projectcontroller.cpp 2186d90 
>   shell/runcontroller.cpp 4a5a5e4 
>   shell/uicontroller.cpp 2c0400f 
>   sublime/area.h 878c120 
>   sublime/area.cpp df29ce3 
>   sublime/mainwindow.h 96b9e71 
>   sublime/mainwindow.cpp f405200 
>   sublime/mainwindow_p.h 7885d06 
>   sublime/mainwindow_p.cpp 23c638d 
> 
> Diff: http://git.reviewboard.kde.org/r/112231/diff/
> 
> 
> Testing
> -------
> 
> Been using it for a couple of days, seems safe.
> 
> 
> File Attachments
> ----------------
> 
> 
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/08/23/pairs-credits2.png
> 
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/08/23/pairs-credits2_1.png
> 
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/08/23/pairs-credits2_2.png
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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


More information about the KDevelop-devel mailing list