Review Request: Replace Projects QComboBox by Filtered QListView in KDE Provider

[Andreas Pakulat]

> On Dec. 11, 2011, 9:50 p.m., Aleix Pol Gonzalez wrote:
> > providers/kdeprovider/kdeproviderwidget.cpp, line 116
> > <http://git.reviewboard.kde.org/r/103383/diff/1/?file=43094#file43094line116>
> >
> >     Is it really possible that the index is not valid? Doesn't look like it...

Maybe this cannot happen today, but you never know what code-changes make it possible tomorrow. A function taking some input should always check the inputs validity as much as it can. In this particular case you otherwise risk crashes due to asserts in the index if row/column etc. are being accessed.


> On Dec. 11, 2011, 9:50 p.m., Aleix Pol Gonzalez wrote:
> > providers/kdeprovider/kdeproviderwidget.cpp, line 93
> > <http://git.reviewboard.kde.org/r/103383/diff/1/?file=43094#file43094line93>
> >
> >     This shouldn't be needed, if it is it means that we let the user to select, nothing, which is bad UI-wise.

Same as for the lower comment. Even if the pos is no input, it may happen that codepaths change in the future making a call of this function possible when there's no current index yet. Also what is the initial selection after opening the dialog?


On Dec. 11, 2011, 9:50 p.m., David Narváez wrote:
> > All in all I think it's good, I'd like to see less gray space in the UI though. What about moving the "Settings" by the filter?
> > Maybe you can store last used projects in the filter.
> > 
> > I think that the combo box worked fine. If it doesn't fit and we want to improve further, we have to do it thoroughly.

If the settings have something to do with the list I agree it would be better to move them up to level with the lineedit and expand the listview.

Regarding 'combobox worked fine', I don't think it does. A combobox is ok if you have a couple of entries in it, if you cannot
know the amount of content a listview with a filter-lineedit is much more useful since its easier to find the thing you want. All you can do in a combobox is scroll around - even if its sorted thats cumbersome compared to typing 3 unique letters of the target and have the list collapsed to a handful of entries.


- Andreas


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


On Dec. 11, 2011, 7:29 a.m., David Narváez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103383/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2011, 7:29 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> This proposal is very similar to review 103187, it should help developers find projects faster.
> 
> 
> Diffs
> -----
> 
>   providers/kdeprovider/kdeproviderwidget.h 8025638 
>   providers/kdeprovider/kdeproviderwidget.cpp eb84faf 
> 
> Diff: http://git.reviewboard.kde.org/r/103383/diff/diff
> 
> 
> Testing
> -------
> 
> Fetched Amarok from the KDE Provider. See new UI in attached image.
> 
> 
> Thanks,
> 
> David Narváez
> 
>

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