Closed afzalsayed96 closed 1 year ago
Latest commit: ee0f83d7ae95c28d5c196f65399fe5109e0c9c1a
The changes in this PR will be included in the next version bump.
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Updated |
---|---|---|---|
evidence-docs | ✅ Ready (Inspect) | Visit Preview | Nov 11, 2022 at 4:49PM (UTC) |
@afzalsayed96 thanks for looking into this!
I tried your changes and got the resizing working when I use client-side navigation, but ran into a couple of issues - do these also show up on your end, or are you seeing different behaviour?
The chart gets into the correct width eventually, but still starts in the extra-wide position
The "finished" event looks like it continues to fire after the chart has finished rendering
console.log("resizing")
to the resize function to see how often it was firing@hughess It seems the root cause of the issue as pointed in https://github.com/apache/echarts/issues/11791 is related to flex layout of the container. And the solution is to observe the flex child for resize events.
While, this solution does seem better than the previous one, it relies on the assumption that class 'chart-container'
would be always applied. Let me know what you think of it.
@afzalsayed96 this seems to work perfectly! Nice solution.
I don’t think there’s an issue with relying on the chart container since it should always be there.
There are two last pieces to think about on this:
@afzalsayed96 I've tested with some charts I set up to be less than 100% width and it works well.
After taking a closer look, I have question about this line:
const chartContainerElement = document.getElementsByClassName('chart-container')[0]
If a page has multiple charts, this will only select the first chart on the page - is that okay in this case because we only need to observe one chart on the page to effectively trigger the resizing for all the charts?
If a page has multiple charts, this will only select the first chart on the page - is that okay in this case because we only need to observe one chart on the page to effectively trigger the resizing for all the charts?
Yes, incidentally this would have worked, as when one chart-container
would resize due to it's parent container, it's siblings would resize as well. But it wasn't that clean after all.
Hence, I've switched the logic to observe the parent article
element which triggers resize of it's children chart-container
elements. This will have the same effect and is more explicit.
That's perfect. I'd say this is good to go! Going to merge into main.
Thanks again for this contribution @afzalsayed96!!
Hey @afzalsayed96,
Was just playing around with a site on mobile (iOS, Firefox, Evidence v5.0.16) and noticed that on some charts data were not being loaded until I interacted with the page:
Platforms:
Could this be related the this PR, do you think?
Hey Archie,
Let me take a look at this later tonight and get back to you
On Wed, Dec 7, 2022 at 10:08 AM Archie @.***> wrote:
Hey @afzalsayed96 https://github.com/afzalsayed96,
Was just playing around with a site on mobile (iOS, Firefox), and noticed that some charts were not being loaded until I interacted with the page:
Could this be related the this PR, do you think?
— Reply to this email directly, view it on GitHub https://github.com/evidence-dev/evidence/pull/450#issuecomment-1340363141, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLBEO5DEVWDNE3RO246AUDWMAIEHANCNFSM6AAAAAARWKQCYM . You are receiving this because you were mentioned.Message ID: @.***>
Source code I used to generate this: https://github.com/evidence-dev/demo/tree/add-customer-reporting-example
Deploy preview: https://deploy-preview-6--evidence-demo.netlify.app/partners
I was playing around and found an issue related to chart layout when performing client side navigation
https://user-images.githubusercontent.com/14029371/199779439-3b1c19a1-0d19-408e-980d-275415adebd1.mov
I'm new using echarts so not sure if this would be the most idiomatic solution to this problem. Happy to hear your thoughts