Redraw chart when its modal or accordion pane is shown - #3135
Open
wakqasahmed wants to merge 2 commits into
Open
Redraw chart when its modal or accordion pane is shown#3135wakqasahmed wants to merge 2 commits into
wakqasahmed wants to merge 2 commits into
Conversation
The chart is built at construction time via new Chart(this.data.get("parent"), ...),
which sizes itself from the container's current width. Inside a modal or accordion
pane that's still display:none at that point, it gets built at zero width, and the
existing shown.bs.tab listener never covers this case since neither container is a
tab.
Wires shown.bs.modal / shown.bs.collapse on the closest .modal / .collapse ancestor,
same pattern as the tab listener, calling the existing debounced draw().
Fixes orchidsoftware#3089
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The chart is constructed at
new Chart(this.data.get("parent"), ...)inconnect(), which sizes itself off the container's width at that moment. Inside a modal or an accordion pane that's stilldisplay:none, that's zero — the SVG comes out withwidth="0"and garbage path coordinates. The existingshown.bs.tablistener already redraws it for the tab case, but nothing covers a modal or a collapse, matching what's reported: only the chart that happens to be visible on load renders correctly.Same fix as the tab one: find the closest
.modal/.collapseancestor and redraw onshown.bs.modal/shown.bs.collapse.Reproduced this outside the app with the actual
frappe-chartsbuild and a real Bootstrap 5 modal/collapse — confirmed a chart built while hidden renders withwidth="0"and negative/garbage path coordinates, and that firing the same redraw this PR adds fixes both cases (widthcomes back correct and the path matches a chart built while visible). No JS test harness exists in this repo for Stimulus controllers (same as #3130), so that browser check is what I verified against instead of an automated test.(Replaces #3134, which had the wrong commit author — closing that one.)
Fixes #3089