Skip to content
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

add fastdom to batch reads and writes to the DOM #170

Merged
merged 2 commits into from
Sep 28, 2023
Merged

add fastdom to batch reads and writes to the DOM #170

merged 2 commits into from
Sep 28, 2023

Conversation

halvtomat
Copy link
Contributor

@halvtomat halvtomat commented Sep 26, 2023

Add fastdom to increase customMarker performance by batching DOM reads and writes.

Original PR #163
Closes #162

@socket-security
Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
fastdom 1.0.11 None +1 71.3 kB wilsonpage

src/utils/index.ts Show resolved Hide resolved
@halvtomat
Copy link
Contributor Author

I noticed that a new bug is introduced by this change, all markers flicker once on the dragEnd event. Have tried to find the cause of this without luck. IMO the performance is worth the small annoyance that the flicker causes but it would be best to find and remove the bug as well.

@HusamElbashir
Copy link
Collaborator

I noticed that a new bug is introduced by this change, all markers flicker once on the dragEnd event. Have tried to find the cause of this without luck. IMO the performance is worth the small annoyance that the flicker causes but it would be best to find and remove the bug as well.

It's obviously less than ideal but if you can't fix the flicker can you at least share a reproduction so we can have that documented? You can fork this sandbox and point it at your fork/branch:
https://codesandbox.io/p/sandbox/confident-haze-xq7wgh?file=package.json
Note that you'll need the latest from develop. @Tofandel added a prepare script so you could install directly from git.

@halvtomat
Copy link
Contributor Author

Here is a reproduction of the flicker. https://codesandbox.io/p/sandbox/hardcore-pike-9dcgzn
It only happens on the dragEnd event.

@HusamElbashir
Copy link
Collaborator

OK this is more noticeable than I had hoped. I'll accept the PR for now but it will have to wait for a fix before we can release it. Might have to put it behind a enableBatchMode feature flag or similar if it takes too long to fix.

@HusamElbashir HusamElbashir merged commit cd405e9 into inocan-group:develop Sep 28, 2023
@HusamElbashir
Copy link
Collaborator

Thanks for the contribution @halvtomat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CustomMarker draw method forces layout reflow
2 participants