Skip to content

Commit

Permalink
fix: zoom rerendering
Browse files Browse the repository at this point in the history 
Co-authored-by: YannNeobards <YannNeobards>
  • Loading branch information
@charlieforward9
charlieforward9 committed Dec 13, 2024
1 parent 796eb4e commit 0bd9217
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/canvas-overlay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export class CanvasOverlay extends Layer {
}
pane.appendChild(this.canvas);

map.on("zoom", this._reset, this);
map.on("moveend", this._reset, this);
map.on("resize", this._resize, this);

Expand All @@ -130,6 +131,7 @@ export class CanvasOverlay extends Layer {
pane.removeChild(this.canvas);
}

map.off("zoom", this._reset, this);
map.off("moveend", this._reset, this);
map.off("resize", this._resize, this);

Expand Down

6 comments on commit 0bd9217

@utilmind
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is odd and duplication. "moveend" is already triggering after "zoom".

@charlieforward9
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#132 (comment)

See this issue I was having with zooming. This does not entirely duplicate.

@utilmind
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charlieforward9 ok, I think I'm confusing it with "zoomend". Yes, "zoom" is different. And there is "viewreset" event too, triggered when animation stops.

I had similar issue to what you pointed in #132 comment when I drawn Glify points on L.Renderer layer. I simply switched to L.Layer and this solved all my issues with zoom animation. As far as I remember, their main difference is animation CSS.

@charlieforward9
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@utilmind Yeh no worries. If you have any new features to contribute I'd love to review them and add them in to make this package better.

@trafficonese
Copy link
Contributor

@trafficonese trafficonese commented on 0bd9217 Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just on beta or?
Can we merge beta to master? @charlieforward9

I think we already had that in master, but the you merged my PR and I didnt have the latest stage..? Or am I confusing things?

@charlieforward9
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trafficonese I believe this is just on beta. If yuou review the PR for me, I can merge it in immediately upon approval.

Thanks!

Please sign in to comment.