display reconciliation date on homepage
ClosedPublic

Authored by mhubner on Jan 20 2018, 10:53 PM.

Details

Summary

this patch adds the possibility to have the corrresponding reconciliation date displayed alongside each account on the home page. This is optional and can be enabled or disabled via a setting in the config dialog.

Diff Detail

Repository
R261 KMyMoney
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mhubner requested review of this revision.Jan 20 2018, 10:53 PM
mhubner created this revision.
mhubner retitled this revision from this patch adds the possibility to have the corrresponding reconciliation date displayed alongside each account on the home page. This is optional and can be enabled or disabled via a setting in the config dialog. to display reconciliation date on homepage.Jan 20 2018, 10:54 PM
mhubner edited the summary of this revision. (Show Details)
mhubner added a project: KMyMoney.
tbaumgart requested changes to this revision.Jan 21 2018, 2:54 PM
tbaumgart added a subscriber: tbaumgart.

For some reason, master crashes when starting. This is not related to your patch and maybe caused by my local environment, but I cannot take a look at the changes from a visual point of view. I wonder if the new column is a bit wide. Please take a look at my other comments.

kmymoney/views/khomeview_p.h
261

Use

const auto lastReconciliationDate = ...

here

This revision now requires changes to proceed.Jan 21 2018, 2:54 PM

I like this. I have been thinking about filing a wish list for it for some time.

I do agree with Thomas that the column seems too wide, but in English, it is because of "Reconciliation." Personally, I would make the column title "Last Reconcile" or "Last Reconciled" which would be narrower, even if it isn't completely grammatically correct.

I also think the "Current Balance" column is wider than necessary, but I assume that is unrelated to this patch, and might be looked at in the future.

mhubner updated this revision to Diff 25732.Jan 22 2018, 12:06 AM

incorporated feedback from Thomas and Jack.

mhubner marked an inline comment as done.Jan 22 2018, 12:08 AM

This looks better to me. Hopefully Thomas can also check it after solving his crash issue.

wojnilowicz accepted this revision as: wojnilowicz.Jan 23 2018, 5:30 PM
wojnilowicz added a subscriber: wojnilowicz.

If others will accept visual aspect of the patch, then it's good for me too.

tbaumgart accepted this revision.Mar 10 2018, 1:43 PM
This revision is now accepted and ready to land.Mar 10 2018, 1:43 PM

Would anyone with write access to the repo mind taking care of the merge, please?
Thanks!

This revision was automatically updated to reflect the committed changes.