[IMP] charts: improve runtime generation lisibility #5105
+989
−344
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The code that ensure that the the trend lines points in a chart have no tooltip title is uselessly complex, with reference to some "originalTooltipTitle" and a //@ts-expect-error. In fact, originalTooltipTitle is almost always undefined, except for linear charts where it's a simple callback that return an empty string. This commit simplifies the code, removing the obscure reference to "originalTooltipTitle" and the //@ts-expect-error. The callback now simply returns an empty string where we do not want a title, and undefined if we want to let ChartJs do it's thing. Task: 4269632
|
|
The trend line of area charts should not have a filled area under the line. Also the trend line configuration should probably not inherit all the values of their parent dataset, as it's easy to miss thing and have options that make no sense (pointBackgroundColor being the color of the parent dataset rather than the color of the trend line for example). Task: 4274294
This commit removes the test `creating chart with single dataset should have legend position set as none, followed by changing it to top`. The test is bad because: - contrary to what the test name suggests, the first thing it does is creating a chart with TWO dataset - it test that legend.display is undefined which suggest that there is no legend, but chartJS default is to display the legend when this property is undefined. So the chart actually has a legend. - the last thing it does is ... testing that the legend is updated when the chart is updated ? I'm not sure how it relates with the supposed first part, and it's already tested elsewhere - it also does an useless and random nextTick(), on a test with zero UI elements nor async code Task: ?
hokolomopo
force-pushed
the
master-chart-ref-dataset-adrm
branch
from
9ee4f53
to
e2ec0d3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
description of this task, what is implemented and why it is implemented that way.
Task: TASK_ID
review checklist