<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/105691/">http://git.reviewboard.kde.org/r/105691/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 30th, 2012, 5:45 p.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On August 7th, 2012, 3:20 p.m., <b>Ivan Shapovalov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Could you please check this?</pre>
</blockquote>
<p>On August 7th, 2012, 7:17 p.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Heh, I've immediately hit the assert in line 236 in custombuildsystemconfigwidget :( So I guess I should've actually tried the previous patches.</pre>
</blockquote>
<p>On August 7th, 2012, 7:39 p.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On August 8th, 2012, 8:53 a.m., <b>Ivan Shapovalov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On August 8th, 2012, 6:05 p.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On August 9th, 2012, 12:45 p.m., <b>Ivan Shapovalov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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").</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 :)</pre>
<br />
<p>- Andreas</p>
<br />
<p>On August 9th, 2012, 12:36 p.m., Ivan Shapovalov wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop and Andreas Pakulat.</div>
<div>By Ivan Shapovalov.</div>
<p style="color: grey;"><i>Updated Aug. 9, 2012, 12:36 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Manual testing.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>configwidget.h <span style="color: grey">(ce9ba32)</span></li>
<li>configwidget.cpp <span style="color: grey">(38bc551)</span></li>
<li>configwidget.ui <span style="color: grey">(d411d83)</span></li>
<li>custombuildsystemconfig.h <span style="color: grey">(62901d4)</span></li>
<li>custombuildsystemconfigwidget.h <span style="color: grey">(d94e9fc)</span></li>
<li>custombuildsystemconfigwidget.cpp <span style="color: grey">(791b7c7)</span></li>
<li>includesmodel.h <span style="color: grey">(debf278)</span></li>
<li>includesmodel.cpp <span style="color: grey">(ab42e16)</span></li>
<li>kcm_custombuildsystem.cpp <span style="color: grey">(2f222bf)</span></li>
<li>projectpathsmodel.h <span style="color: grey">(d32bb92)</span></li>
<li>projectpathsmodel.cpp <span style="color: grey">(6ee35b7)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105691/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/105691/s/654/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/07/30/old_ui_400x100.png" style="border: 1px black solid;" alt="Before changes" /></a>
<a href="http://git.reviewboard.kde.org/r/105691/s/655/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/07/30/new_ui_idle_400x100.png" style="border: 1px black solid;" alt="New UI - idle" /></a>
<a href="http://git.reviewboard.kde.org/r/105691/s/656/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/07/30/new_ui_idle_editing_400x100.png" style="border: 1px black solid;" alt="New UI - editing project path" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>