- User Since
- Apr 28 2020, 8:28 AM (5 w, 2 d)
Fri, May 29
Mon, May 18
Sat, May 16
Added the fix for incorrect Next button tooltip messages on the Schedule page.
Fri, May 15
OK, I found it.
The 2nd last line of the isComplete() method should be d->m_wizard->d_func()->m_nextButton->setToolTip(msg); instead of d->m_wizard->d_func()->m_finishButton->setToolTip(msg);
Now the tooltip message informs the user why the Next button is disabled.
@tbaumgart, That's right. I also investigated those #if o #endif lines to find out why were they added before I decided to bring those two payment methods back and found no good reason. It might well have been only temporary and the author forgot to revert the change.
@ostroffjh, I agree that your concerns are important. I have just skimmed through the code and came acrosss something that may render useful.
There is a virtual method isComplete() in KMyMoneyWizardPage class.
- This returns, if all necessary data for this page has been
- filled. It is used to enabled the 'Next' or 'Finish' button.
- The button is only enabled, if this method returns @p true,
- which is the default implementation. *
- @retval false more data required from the user before we can proceed
- @retval true all data available, we allow to switch to the next page */ virtual bool isComplete() const;`
The method is overridden in subclass CreditCardSchedulePage which is changed by this patch.
It seems this approach works as expected on the previous page:
For some reason tooltip message is not supported correctly on the Schedule page. This needs looking into more closely to find out why.
The patch also disallows the user to intentionally set empty payment method. Empty method is not listed. Therefore I consider the fix quite solid.
Wed, May 13
@tbaumgart, thank you for your acceptance. Could you please land the patch for me? I do not have git push permission.
Mon, May 11
Sun, May 10
Fri, May 8
Removed unnecessary function declaration.
Added the requested changes.
Wed, May 6
Thanks :) I really like to see that charts work as expected.
Simlified c'tor arguments. Removed unnecessary if statement. Added identation.
Tue, May 5
Fixed typos in the commit message
May 1 2020
With the priceless help from @tbaumgart I managed to find a solution. Please find attached the final version of the patch.
I have done some more testing and I realized that precision (number of digits after the decimal point) is subject to change. In fact, account balance history chart is pretty straightforward but the code that I worked with is also used with configurable charts in reports. Precision can be changed in the Range tab of the Report Configuration window.
My previous patch created yAxis by calling new KReportCartesianAxis(loc, m_precision) whereas it should be new KReportCartesianAxis(loc, config.yLabelsPrecision()).
Apr 30 2020
0.0 is quite a special argument for the logarithm. I made one last change adding support for this case.
Also there was a todo comment in the code I have removed, so thought it to be appropriate to take care of it lest it be missed out.
/ @todo this might also need some vertical adjustment in case of a horizontal line
/ see below how this has been solved for linear graphs
Indeed, support for logarithmic axis type was required.
Now I think the code is robust and complete. Now I'll just wait for the final review from you :)
Apr 29 2020
There is one thing I overlooked in my previous revision. I have just tested my changes against a new account with clear history (empty ledger). It leads to a horizontal-line chart that is drawn incorrectly if no custom grid extension is made. The conclusion is the original grid extension code is here to stay. I have just moved it from void KReportChartView::slotNeedUpdate() to the bottom of void KReportChartView::drawPivotChart( ... ) and I refactored a little bit. Sorry for messing around in this already accepted patch. The good thing is labels are now printed correctly even when one zooms a horizontal-line chart :)
Thank you, @tbaumgart for your time spent reviewing my changes. I have amended my commit message with tag "BUG: 420767"
Apr 28 2020
Uploaded a new diff patch after changes according to @tbaumgart's comments.