Created attachment 128010 [details] Zoomed account balance chart SUMMARY When one zooms a chart (using so-called rubber band zooming feature from KDiagram project) axis ticks and labels are recalculated but they are overridden with customized "hard labels". The number of axis ticks is increasing and becomes grater than the count of hard labels, so the labels are repeated in cycle. In and of itself, it is really a feature rather than a bug but the feature works to our disadvantage in this case producing incorrect ordinate axis labels. STEPS TO REPRODUCE 1. Open any account ledger and click on "Show balance chart" button at the top. 2. Click and drag your left mouse button to zoom boxed area. OBSERVED RESULT Ordinate axis labels are incorrect and repeated in cycle. EXPECTED RESULT Ordinate axis labels should match zoomed chart's data. SOFTWARE/OS VERSIONS KDE Plasma Version: 5.18.4 KDE Frameworks Version: 5.69.0 Qt Version: 5.14.2 ADDITIONAL INFORMATION Please find attached a screenshot presenting the issue.
Created attachment 128011 [details] Newly opened account balance chart before zooming
Git commit 5e4c8322fbefcb6cdaf1c51f6204986946ca0ba4 by Thomas Baumgart, on behalf of Robert Szczesiak. Committed on 02/05/2020 at 05:11. Pushed by tbaumgart into branch 'master'. Fix ordinate axis labels when zooming Summary: If I understand the intent of the author correctly, the sole purpose of `KReportChartView::slotNeedUpdate()` was one-time customization of ordinate axis labels at the time of chart creation. The fact that signal is disconnected immediately after execution of slot starts seems to confirm my understanding. The problem begins when one zooms the chart (using so-called rubber band zooming feature from KDiagram project). Axis ticks and labels are recalculated but still they are overridden with "hard labels" defined in the slot body. The number of axis ticks increases and is grater than the number of hard labels, so the labels are repeated in cycle. In and of itself it is really a feature rather than a bug but the feature works to our disadvantage in this case. Please find below a quote from AbstractAxis Class documentation: > void AbstractAxis::setLabels( const QStringList & list ) > Use this to specify your own set of strings, to be used as axis labels. > Labels specified via setLabels take precedence: If a non-empty list is passed, KD Chart will use these strings as axis labels, **instead of calculating them.** > **If you pass a smaller number of strings than the number of labels drawn at this axis, KD Chart will repeat the strings until all labels are drawn**. As an example you could specify the seven days of the week as abscissa labels, which would be repeatedly used then. Unfortunately, mere removal of disconnect method call does not solve the problem but the same documentation provides a way to customize axis labels when needed. It is done through overriding of `const QString AbstractAxis::customizedLabel( const QString & label ) const` My solution proposed in attached patch goes this way and also simplifies calculations where they seemed over-complicated to me. Please correct my understanding if i missed the point of removed lines. Please find attached screenshots before and after the fix. Balance history chart before fix with no zoom: {F8268351} Balance history chart before fix with zoom:: {F8268353} Balance history chart after fix with no zoom: {F8268359} Balance history chart after fix with zoom:: {F8268358} As a footnote, I would like to sincerely thank to all the people who have contributed to this project. I started to use KMyMoney recently and I am surprised how good it is, especially for an open-source project run by the community. It suits my need for personal finance and investment-supporting software amazingly well. If you find my contribution valuable, I would love to provide more patches in the future if I find more bugs or feel that some feature is missing. In fact, I have little to no industrial experience in C++ as I work mostly in Java. Please let me know if I broke any C++ coding rules. Reviewers: #kmymoney, tbaumgart Reviewed By: #kmymoney, tbaumgart Subscribers: tbaumgart Tags: #kmymoney, #kdiagram Differential Revision: https://phabricator.kde.org/D29255 M +1 -0 kmymoney/plugins/views/reports/core/CMakeLists.txt A +57 -0 kmymoney/plugins/views/reports/core/kreportcartesianaxis.cpp [License: GPL (v2+)] A +48 -0 kmymoney/plugins/views/reports/core/kreportcartesianaxis.h [License: GPL (v2+)] M +26 -56 kmymoney/plugins/views/reports/core/kreportchartview.cpp M +6 -2 kmymoney/plugins/views/reports/core/kreportchartview.h M +1 -1 kmymoney/plugins/views/reports/reporttabimpl.cpp https://commits.kde.org/kmymoney/5e4c8322fbefcb6cdaf1c51f6204986946ca0ba4
Git commit 332b888252b4bd8e08b9d21e21d34e5be3ac25b9 by Thomas Baumgart. Committed on 02/05/2020 at 07:57. Pushed by tbaumgart into branch '5.0'. Fix ordinate axis labels when zooming Summary: If I understand the intent of the author correctly, the sole purpose of `KReportChartView::slotNeedUpdate()` was one-time customization of ordinate axis labels at the time of chart creation. The fact that signal is disconnected immediately after execution of slot starts seems to confirm my understanding. The problem begins when one zooms the chart (using so-called rubber band zooming feature from KDiagram project). Axis ticks and labels are recalculated but still they are overridden with "hard labels" defined in the slot body. The number of axis ticks increases and is grater than the number of hard labels, so the labels are repeated in cycle. In and of itself it is really a feature rather than a bug but the feature works to our disadvantage in this case. Please find below a quote from AbstractAxis Class documentation: > void AbstractAxis::setLabels( const QStringList & list ) > Use this to specify your own set of strings, to be used as axis labels. > Labels specified via setLabels take precedence: If a non-empty list is passed, KD Chart will use these strings as axis labels, **instead of calculating them.** > **If you pass a smaller number of strings than the number of labels drawn at this axis, KD Chart will repeat the strings until all labels are drawn**. As an example you could specify the seven days of the week as abscissa labels, which would be repeatedly used then. Unfortunately, mere removal of disconnect method call does not solve the problem but the same documentation provides a way to customize axis labels when needed. It is done through overriding of `const QString AbstractAxis::customizedLabel( const QString & label ) const` My solution proposed in attached patch goes this way and also simplifies calculations where they seemed over-complicated to me. Please correct my understanding if i missed the point of removed lines. Please find attached screenshots before and after the fix. Balance history chart before fix with no zoom: {F8268351} Balance history chart before fix with zoom:: {F8268353} Balance history chart after fix with no zoom: {F8268359} Balance history chart after fix with zoom:: {F8268358} As a footnote, I would like to sincerely thank to all the people who have contributed to this project. I started to use KMyMoney recently and I am surprised how good it is, especially for an open-source project run by the community. It suits my need for personal finance and investment-supporting software amazingly well. If you find my contribution valuable, I would love to provide more patches in the future if I find more bugs or feel that some feature is missing. In fact, I have little to no industrial experience in C++ as I work mostly in Java. Please let me know if I broke any C++ coding rules. Reviewers: #kmymoney, tbaumgart Reviewed By: #kmymoney, tbaumgart Subscribers: tbaumgart Tags: #kmymoney, #kdiagram Differential Revision: https://phabricator.kde.org/D29255 (cherry picked from commit 5e4c8322fbefcb6cdaf1c51f6204986946ca0ba4) M +1 -0 kmymoney/plugins/views/reports/core/CMakeLists.txt A +57 -0 kmymoney/plugins/views/reports/core/kreportcartesianaxis.cpp [License: GPL (v2+)] A +48 -0 kmymoney/plugins/views/reports/core/kreportcartesianaxis.h [License: GPL (v2+)] M +26 -56 kmymoney/plugins/views/reports/core/kreportchartview.cpp M +6 -2 kmymoney/plugins/views/reports/core/kreportchartview.h M +1 -1 kmymoney/plugins/views/reports/reporttabimpl.cpp https://commits.kde.org/kmymoney/332b888252b4bd8e08b9d21e21d34e5be3ac25b9