-
Notifications
You must be signed in to change notification settings - Fork 68
[FIX] Charts: Ensure Chart js extension are loaded on chart creation #7378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: saas-18.2
Are you sure you want to change the base?
Conversation
|
This PR targets the disabled branch odoo/o-spreadsheet:saas-18.1, it needs to be retargeted before it can be merged. |
c142f5e to
50e6570
Compare
|
This PR targets the disabled branch odoo/o-spreadsheet:saas-18.1, it needs to be retargeted before it can be merged. |
50e6570 to
cd4a145
Compare
|
This PR targets the disabled branch odoo/o-spreadsheet:saas-18.1, it needs to be retargeted before it can be merged. |
cd4a145 to
4ee15c7
Compare
|
This PR targets the disabled branch odoo/o-spreadsheet:saas-18.1, it needs to be retargeted before it can be merged. |
|
@robodoo help |
|
Currently available commands for @rrahir:
Note: this help text is dynamic and will change with the state of the PR. |
|
robodoo fw=no |
|
Disabled forward-porting. |
hokolomopo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋
| if (!extensionsLoaded) { | ||
| unregisterChartJsExtensions(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems wrong no ? this will unload the chartJExtensions if they were loaded before calling this helper, and we only converted gauge/scorecards. The load/unload should probably be fully inside the if("chartJsConfig" in runtime)
When calling the method chartToImage, the chartJs extensions might not be loaded as we load them conditionally when mounting a chart. Hence, calling `ChartToImage` when there are no visible charts in the viewport or if we just instantiate a model without loading a component `Spreadsheet`, the extensions will be missing. Right now, the plugins/extensions are not fundamental to convert a chart to an image but this becomes problematic with the arrival of future charts (for instance Funnel). This commit adds a check to ensure that the extensisons are loaded and unloads them after use as they can cause crashes when using the global `ChartJs` variable elsewhere (see #6076). Task: 5214007
4ee15c7 to
7379762
Compare

When calling the method chartToImage, the chartJs extensions might not be loaded as we load them conditionally when mounting a chart. Hence, calling
ChartToImagewhen there are no visible charts in the viewport or if we just instantiate a model without loading a componentSpreadsheet, the extensions will be missing.Right now, the plugins/extensions are not fundamental to convert a chart to an image but this becomes problematic with the arrival of future charts (for instance Funnel).
Task: 5214007
Description:
description of this task, what is implemented and why it is implemented that way.
Task: TASK_ID
review checklist