Review Request: [17/18] Finally refactor main UI

Thu Aug 9 20:54:37 UTC 2012

> On July 30, 2012, 5:45 p.m., Andreas Pakulat wrote:
> > Hmm, I think I might like this change provided I can still in-line edit (personally I don't need an inline-url-selector) the entries. I'd still like to try it out in action before merging it, lets re-visit this one once the other ones it depends on are merged completely.
> 
> Ivan Shapovalov wrote:
>     Could you please check this?
> 
> Andreas Pakulat wrote:
>     Heh, I've immediately hit the assert in line 236 in custombuildsystemconfigwidget :( So I guess I should've actually tried the previous patches.
> 
> Andreas Pakulat wrote:
>     Ok, that was not related to this patch. I've played a bit with the new UI and there are a few things:
>     
>     - I don't like the semantic change of the buttons when adding a new directory to the list of paths. It was very surprising how this worked for me.
>     
>     - Why is there Edit and the url-selector button to start editing, one of them should be enough IMHO.
>     
>     - I'm not sure I like the extra "Add" buttons and all the inline-stuff. Maybe add should just pop up a dialog instead with the url-selector?
>     
>     - The url-selector for the path starts in the parent of the project root and has the project dir selected. Not sure wether its possible to have this but I think it would be better to be inside the project directory.
>     
>     - the path-Remove should be disabled when the project-root is selected or it should be possible to remove the project-root. Something is not working as it should there.
>     
>     - The remove-selection button does not work correctly, it removed more than it should have. I've added a new entry and then selected it and clicked remove. All non-project paths where removed (there were 2). This may be related to an earlier change though about storing relative paths.
>     
>     But all that aside, I think the overall ui-rework is good.
> 
> Ivan Shapovalov wrote:
>     Ugh. Sorry for that crash; I should've test changes more exhaustively...
>     
>     Regarding this:
>     
>     > Edit and url-selector button
>     I thought that one may want to correct some character-local typo and it's unnecessary in that case to fire up a dialog... Initially I've wanted to handle double-click on the combobox, but that turned out to be impossible. But ok, it's easy to remove an extra button.
>     
>     > semantic change of the buttons
>     I'll make them static; there will be initially only an "Add" button which adds an empty path entry, switches to it and enables editing. Sounds good?
>     
>     > inline-stuff
>     What are you talking about? Project pathes' combobox or includes view?
>     
>     Everything else I'll fix.
> 
> Andreas Pakulat wrote:
>     I think the 'inline-stuff' refers to the way the path-combobox and how it behaves. Things I don't like about it are:
>     
>     - The buttons-change as I said before
>     - When editing/adding a url, its still a url-combobox but I don't think it makes sense to select some other entry in this case
>     - One can click the url-selector-button to get the filedialog, select "something" and then sees Add/Save/Cancel which is confusing since its unclear what the difference between Add and Save is.
>     
>     What do you think about using the same ui for both cases, except having a combobox for the path? Something like this (sorry for the ascii art):
>     
>     [url-selector]                                    [Add] [Replace]
>     [Combobox]                                        [Remove]
>     
>     [Includes/Imports] [Defines]
>     -----------------------------------------------------------------
>     [url-selector]                                    [Add] [Replace]
>     --------------------------------------------------------
>     | path1                                                | [Remove]
>     |                                                      |
>     --------------------------------------------------------
>     
>     Where the selecting something in the combobox or the listview sets the url-selector to that path. The user can change the entry and click "Add" to add a new entry to the combobox or the lower list. Alternatively he can click "Replace" to replace the currently selected entry in the combobox or list with the content of the url-selector. The remove-buttons are 'attached' to the widget they work on. The combobox and url-selectors expand in width as far as possible.
>     
>     Maybe it would be useful to have a really simple small prototype to try out different approaches to this instead of always updating the patch while we find the 'best' solution. Feel free to move the ui-design-discussion to our development list.
>     
>     The include url-selector is ok I think, except that it doesn't clear once I've added an entry.
> 
> Ivan Shapovalov wrote:
>     Ok, I've separated combo-box/URL selector in project pathes and made the buttons to match the picture.
>     The replace/remove buttons are greyed out automatically if there is no current item or the current item is unchangeable (e. g. "project root").

I've added a small app now that shows the dialog. I don't think I'll actually look at the changes today though. Maybe tomorrow or if I'm lucky on the drive to my parents on saturday.

BTW, thanks for being so patient with me and my nitpicks :)

- Andreas

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

On Aug. 9, 2012, 12:36 p.m., Ivan Shapovalov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105691/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2012, 12:36 p.m.)
> 
> 
> Review request for KDevelop and Andreas Pakulat.
> 
> 
> Description
> -------
> 
> Changes done:
> 1) Pass current project to the UI and pathes model.
> -> reasoning: they use it to relativize URLs
> 2) last-item-placeholders in pathes- and includes-model replaced by add*() methods
> -> reasoning: UI will use URL requesters to add new items
> 3) add a persistent (undeleteable; attempts to change its path result in creation of another item) "project root" item to the pathes model
> -> reasoning: usability - many people will want to do common per-project settings
> 4) make project pathes model URL-aware: clean/relativize pathes on adding, introduce custom roles to get full pathes/URLs
> -> reasoning: usability
> 
> UI modifications done:
> 5) replace listview of sub-project pathes with a combobox URL requester
> -> reasoning: less on-screen space
> 6) Replace stacked widget for includes/defines selection with a tab-interface.
> -> reasoning: usability - one can see all choices (includes and defines, maybe language features when they are merged) at once instead of having to open the combobox.
> 7) Use URL requesters and add/remove buttons instead of "Double-Click here..." placeholders.
> -> reasoning: usability - one may select a path without having to type it.
> 8) Add a button to remove selected include/define
> -> reasoning: in comments
> 9) Change various labels and popup texts
> -> reasoning: on-screen readability.
> 10) Clear inner UI on config set rather than before config loading.
> -> reasoning: robustness (when new configuration is selected, UI is cleared and refilled from scratch instead of overwriting old data).
> 
> 
> Diffs
> -----
> 
>   configwidget.h ce9ba32 
>   configwidget.cpp 38bc551 
>   configwidget.ui d411d83 
>   custombuildsystemconfig.h 62901d4 
>   custombuildsystemconfigwidget.h d94e9fc 
>   custombuildsystemconfigwidget.cpp 791b7c7 
>   includesmodel.h debf278 
>   includesmodel.cpp ab42e16 
>   kcm_custombuildsystem.cpp 2f222bf 
>   projectpathsmodel.h d32bb92 
>   projectpathsmodel.cpp 6ee35b7 
> 
> Diff: http://git.reviewboard.kde.org/r/105691/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Screenshots
> -----------
> 
> Before changes
>   http://git.reviewboard.kde.org/r/105691/s/654/
> New UI - idle
>   http://git.reviewboard.kde.org/r/105691/s/655/
> New UI - editing project path
>   http://git.reviewboard.kde.org/r/105691/s/656/
> 
> 
> Thanks,
> 
> Ivan Shapovalov
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120809/2c9c80d3/attachment.html>