Review Request: UI improvements for the project build docker

[Milian Wolff]

> On Nov. 15, 2011, 11:53 a.m., Andreas Pakulat wrote:
> > Sorry but this is wrong:
> > 
> > - There's no need for a proxy model at all, simply do it in the source model
> > - If a proxy is used all places using itemview or selection model indices need to be adjusted to map from source to proxy and vice versa
> > - QProxyModel is obsolete and should not be used in new code
> > 
> > In addition I think this is no improvement at all. If there are two main.cpp files I'd need to hover over each to find out where in the project
> > they are located instead of being able to instantly see it.
> 
> Carlos Licea wrote:
>     > - There's no need for a proxy model at all, simply do it in the source model
>     > - If a proxy is used all places using itemview or selection model indices need to be adjusted to map from source to proxy and vice versa
>     I didn't want to modify the source model for mere aesthetic reasons, hence changing all the users of the original model. I think proxy<->source mapping is unneeded as it's taken care by the proxy class because I'm merely returning a column as tooltip rather than as text, however, if I've broken something, please kindly provide the scenario and I will fix it.
>     
>     >- QProxyModel is obsolete and should not be used in new code
>     I strongly disagree with this deprecation, for this simple case scenario I would think it'd make sense to write a proxy rather than replicate all the logic in the source model, update all the clients for the source model or write a new delegate (which is my fallback currently, if you insist I don't use a proxy model.)
>     
>     >In addition I think this is no improvement at all. If there are two main.cpp files I'd need to hover over each to find out where in the project
>     they are located instead of being able to instantly see it.
>     I don't think the current scenario is much better, the path is highly likely to be much bigger than the pane hence forcing you to scroll horizontally which forces you to use your mouse, if your mouse is in the area anyway why not remove the visual clutter? My intention is to clean up the UI, there are currently way to many horizontal scrollbars.
>     
>     I would argue that the patch provides a cleaner feel to the pane.
> 
> Andreas Pakulat wrote:
>     Proxy <-> source mapping is _always_ needed since the Qt proxy models do not guarantee that just because the proxy does not filter or sort the indices will be the same. It might change them anyway for whatever reasons. Also the only user right now is the projectmanagerview plugin.
>     
>     Regarding the deprecation: I did not talk about proxy models per se, just the class QProxyModel is deprecated in Qt. You should either use QSortFilterProxyModel or QAbstractProxyModel, especially looking forward to Qt5 where QProxyModel will not exist anymore.
>     
>     You're trading having to scroll with having to hover for n seconds and that does not improve the usability at all. And usability is far more important that having some shiny UI - IMHO. If you want to improve things please come up with an easy way to show the path all the time that does not need scrolling. Maybe the buildset widget should be separated into its own view, now that toolviews can be arranged int tabs. (IIRC) That would allow moving it to the bottom area where there is much more horizontal space.
> 
> Milian Wolff wrote:
>     Just want to chime in and say that Andreas is right. Just because your proxy<->source mapping magically works now does not mean it will continue to work in the future, if someone extends the proxy functionality. If one then has to spent time to figure out the reasons just to see that it's due to an improper implementation in the beginning is very frustrating.
> 
> Carlos Licea wrote:
>     Yes, I perfectly understand the problem with my current implementation.
>     However, I'm not sure I can satisfy Andreas' request to always show the path while at the same time fixing what I wanted to fix: visual clutter.
>     I think I'll drop the review request.

could you maybe show screenshots comparing status quo to your new implementation? maybe start a discussion on our mailing list, we could try to find a working, better solution together.


- Milian


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


On Nov. 12, 2011, 8:14 a.m., Carlos Licea wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103115/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2011, 8:14 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> I removed some clutter by removing the path and showing it on a tooltip. I also removed the top header.
> 
> 
> Diffs
> -----
> 
>   plugins/projectmanagerview/CMakeLists.txt 7b58e8e 
>   plugins/projectmanagerview/projectbuildsetproxymodel.h PRE-CREATION 
>   plugins/projectmanagerview/projectbuildsetproxymodel.cpp PRE-CREATION 
>   plugins/projectmanagerview/projectbuildsetwidget.cpp 585a1c8 
>   plugins/projectmanagerview/projectbuildsetwidget.ui 90e74e1 
> 
> Diff: http://git.reviewboard.kde.org/r/103115/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Carlos Licea
> 
>

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