Fix JS expression for chart label typo #30
Conversation
mvorisek
changed the title
|
Michael, again, the PR is still not fixing all issues, see |
|
@mvorisek Can you please stop teaching me what my workflow has to include? I am fixing a bug that was introduced by you which basically results from your own workflow not checking your own code changes sufficiently and introducing in this case new bugs across several scripts. I am assuming this is a joint open-source effort where people are investing their personal time and not an environment for blaming people for not matching your personal quality standards which in this case you have not met yourself. Thank you. |
|
Michael, thank you for the feedback. I was written in that way to stress that I very often feel/felt that you was opening PRs with low efford invested shifting the efford on me. Efford in fixing one line instead of fixing all cases, missing tests... Is this feedback from me clear or what can I do better? Yes, the issue was introduced by me. Of course sorry, it slipped thru my review :) |
src/ScatterChart.php
Outdated
| label += ": "; | ||
| } | ||
| return label + (value ? Number(value).toLocaleString(undefined, {minimumFractionDigits: [], maximumFractionDigits: []}) : "No Data"); | ||
| EOF. |
There was a problem hiding this comment.
dot? do not fix it, I am on this PR now...
|
The changes should be good now, but I have no test env setup for this repo, I would be happy if someone else can review this PR as well. |
Variables in JS expression not processed due to EOF string definition
Effect: Tooltips are no longer working as originally intended.
How to test: Open the
demos/index.phpand check if tooltips work on all the charts. Also there should not be a JS console error as previously an invalid parameter is passed tominimumFractionDigits