-
Notifications
You must be signed in to change notification settings - Fork 17
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
fix(viz): track draw state to apply recenter on fresh draws #740
Conversation
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.
seems to work now. i'm not able to reproduce the issue.
was working on another issue and have to backtrack here. i can reliably reproduce with following steps (on main branch also):
- visit repo builds page
- click on any build to go to its build page
- click "Visualize" tab
it works fine when i hard refresh repo builds page after step 1. and do the remaining steps
fwiw, i'm able to get pretty consistent results it seems by changing two things:
in other words, passing a truthy worst result i saw is an initial flash of uncentered graph and then it fixed itself. seemed to be pretty rare - more rare than the initial issues, i think?! on other minor optimization with this would be to keep track of whether it has already centered and zoomed because it might call it multiple times depending on mix of isrefreshdraw and centerondraw. didn't notice any ill side effects otherwise, but it's possible there's some use case i'm not accounting for. was just trying a few things :) |
@wass3r thank you for taking a look 🙏 i appreciate it
i'll make the other changes, good call |
more stability for the elm vs. js render race condition, where the page root doesnt render quick enough for the graph render dispatch
also
isRefreshDraw
tosameBuild
to make it a bit clearercenterOnDraw
tofreshDraw
to make it a bit clearer