[Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

Sat Apr 19 08:57:28 UTC 2014

I wouldn't say your not a programmer. Tackling that job is only something a 
programmer would do!

So keep it up, Because I certainly appreciate your efforts.

Chris

On 18/04/2014 10:11 PM, Allan Anderson wrote:
> This is an automatically generated e-mail. To reply, visit: 
> https://git.reviewboard.kde.org/r/117620/
>
>
>     On April 18th, 2014, 11:34 a.m. UTC, *Allan Anderson* wrote:
>
>         Hi Cristian
>         Many thanks.  It had been starting to dawn on me that, when dealing with different distros, it shouldn't be necessary to fiddle about with margins
>         to get things to look right.  I'd suspected that I needed to make some fundamental changes, but wasn't really ready to take the plunge.  Anyway,
>         you've done it for me, so again, thank you.
>         The question now is, where do we go from here?
>         Here, I'd already removed a chunk of code and have been working on details of the UI appearance.  For instance, I don't like to see the size of the
>         table changing height between the different wizard pages, and I don't like to see just partial lines displayed.  Obviously, if the user decides to do
>         a resize, then the decision is his, but without that happening, I don't want to see half lines displayed.  This is mainly to do with whether a
>         horizontal scroll-bar is visible or not.  I have not been able to get that to work correctly.  Sometimes, it is visible but isVisible() returns
>         false, and vice versa I think.  So I've had to calculate the diplayed width and adjust the containing rectangle.  A little work is still needed here.
>         In your pruning, I think you have removed some vertical spacers, which is causing the combo-boxes to shift upwards, leaving a lot of space below.
>         I wish to revert that.  In the separators wizard page, the two combo-boxes are now different sizes, I think because you've removed the form layout I was
>         using.  In the banking wizard, the combo-box sizes are significantly different in width between Linux Mint and Ubuntu.  I don't know about on Windows.
>         These are very minor points, but I'd like to tweak them.
>         I'd decided on a default window height of 10 rows, which has been changed.  Was there a particular reason for that?
>         Between Mint and Ubuntu, the margin values are still different, but now that has no effect on the appearance.  The font sizes are differ too, 9 point
>         against 11 point.  That does affect the appearance.  Should I leave that as is, do you think?  I think probably yes.
>         I can't add a revised patch to your review, but if you agree, I can describe any proposed changes.  I don't want to stray away from your major changes,
>         though, obviously.
>
>     On April 18th, 2014, 12:19 p.m. UTC, *Cristian Onet,* wrote:
>
>         "where do we go from here?"
>
>         I don't know, it's up to you to decide. If you ask me (after 2 days of going trough csvdialog.cpp) I would suggest to rewrite everything. I know that seems harsh but frankly I didn't see such "tangled up" code in a while. One reading it couldn't quite tell what is essential (necessary) in there and what is trial/error leftover code.
>
>         For the UI I would have the following guide:
>         - keep it simple (your layouts were really, really complicated)
>         - don't center everything (including messages)
>         - don't set fix sizes (fixed size policies are OK in the appropriate place - line edits, combos should have a fixed vertical size hint)
>         - try to use the available space as uniformly as possible (1 page with 2 combos vs. 1 page with 10 combos)
>         - now we have a dialog (QWizard) inside another dialog (CSVDialog - former widget) which seems a bad idea to me
>
>         For the code:
>         - don't keep everything together (like a big ball of spaghetti), try to break it up into smaller components
>         - only keep the essential code otherwise you won't be able to spot it later from the clutter
>
>         Until the csv importer plugin will not be cleaned up, clean reviewable patches can't be provided because the patches will always look like the code that is patched.
>
>     On April 18th, 2014, 12:21 p.m. UTC, *Cristian Onet,* wrote:
>
>         Forgot to mention the fonts: don't set any fonts - let the user choose the fonts he desires using the system settings.
>
> I've never made any claim to be a programmer!  This all started off as a script to produce a QIF file, which I was advised to expand, first into
> tabs, and later into a wizard.  It has grown, like Topsy.  I've learned a lot along the way, but I'm still not a programmer.
> In the light of your comments, there seems little point now wasting time tuning what we have, so I'll leave it as is.
> When/if a rewrite occurs will depend upon my available time.
>
> - Allan
>
>
> On April 17th, 2014, 9:23 p.m. UTC, Cristian Onet, wrote:
>
> Review request for KMymoney and Allan Anderson.
> By Cristian Onet,.
>
> /Updated April 17, 2014, 9:23 p.m./
>
> *Repository: * kmymoney
>
>
>   Description
>
> Remove fixed layout values from the CSV importer UI.
>
> Also removed a lot of code (a lot still remains) that I think should
> not be in there (like UI control size fine tuning).
>
>
>   Testing
>
> For me the importer looks/feel almost like without this.
>
>
>   Diffs
>
>   * kmymoney/plugins/csvimport/bankingwizardpage.ui
>     (d2179bff0504a67a22efbb364f90e089443d8239)
>   * kmymoney/plugins/csvimport/completionwizardpage.ui
>     (99db07576265b68c764308bbb3da3cae38b151bd)
>   * kmymoney/plugins/csvimport/csvdialog.h
>     (6716ca412db036384d9d8d65f0d1ab40efbe443f)
>   * kmymoney/plugins/csvimport/csvdialog.cpp
>     (3ab7f9537acdb369b0c5b781827564e30d9ad396)
>   * kmymoney/plugins/csvimport/csvdialog.ui
>     (36e7fd51159a1f7391394b5f00bc22387b0bd8f5)
>   * kmymoney/plugins/csvimport/csvimporterplugin.h
>     (d2d2f6ca9aeea0709512afcffbad17abea6a315a)
>   * kmymoney/plugins/csvimport/csvimporterplugin.cpp
>     (602b4d924c8afc0b34819e47b03b88776319bf0c)
>   * kmymoney/plugins/csvimport/introwizardpage.ui
>     (9bd29f5f4339add8790e032f5613ec46c675634d)
>   * kmymoney/plugins/csvimport/investmentwizardpage.ui
>     (3744963e5e9a91ad36b8d0fa78839b296c5810f8)
>   * kmymoney/plugins/csvimport/investprocessing.cpp
>     (d9df90d6b9a6500b499e52ccb3e8734d163765eb)
>   * kmymoney/plugins/csvimport/lines-datewizardpage.ui
>     (fb10a0e228d7bdee9ca1162edcd0d4b69225d68b)
>   * kmymoney/plugins/csvimport/separatorwizardpage.ui
>     (30b2dc7b22ad24b7b2df96332deb25f9c8c76b89)
>
> View Diff <https://git.reviewboard.kde.org/r/117620/diff/>
>
>
>
> _______________________________________________
> KMyMoney-devel mailing list
> KMyMoney-devel at kde.org
> https://mail.kde.org/mailman/listinfo/kmymoney-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20140419/716499a9/attachment.html>