Possibility to refresh GridLayer tiles #9100
Conversation
|
I haven't tested it but the browser cache should break that logic, because it doesn't make a new request. I understand the need and i think if it is possible we should consider implementing a solution for this issue. |
|
I would say 'avoiding browser cache' is responsibility of createTile method, it could be anything, not only 'img', so maybe browser cache is not a problem at all. Responsibility of GridLayer is just to get new element and replace old one. |
|
@Falke-Design @mourner is there a chance to move this PR forward? I am open for discussion |
|
@Falke-Design @mourner still have intention to contribute into your repo =) |
|
@Falke-Design @mourner could you give me the person to contact regarding possibility to review this MR? |
src/layer/tile/GridLayer.js
Outdated
| @@ -891,6 +908,27 @@ export const GridLayer = Layer.extend({ | |||
| } | |||
| }, | |||
|
|
|||
| _tileUpdate(tile, err, el) { | |||
There was a problem hiding this comment.
This method is used only once (in refreshTiles()) and therefore I'd be tempted to hoist it in there, converting it into an anonymous function.
There was a problem hiding this comment.
I tried to copy codestyle used in the code around, but no problem, done (hope I understood you right)
src/layer/tile/GridLayer.js
Outdated
| tile.el = el; | ||
| tile.loaded = +new Date(); | ||
|
|
||
| this.fire('tileupdate', { |
There was a problem hiding this comment.
This event needs docstrings.
src/layer/tile/GridLayer.js
Outdated
| @@ -272,6 +272,17 @@ export const GridLayer = Layer.extend({ | |||
| return document.createElement('div'); | |||
| }, | |||
|
|
|||
| // @method refreshTiles: void | |||
| // Triggers creating of new tile HTML element for each existing tile | |||
There was a problem hiding this comment.
Compare with the redraw method. Give users some explanation (in the docstrings) why they should use one or the other.
src/layer/tile/GridLayer.js
Outdated
| } | ||
|
|
||
| el.classList.add('leaflet-tile-loaded'); | ||
| tile.el.replaceWith(el); |
There was a problem hiding this comment.
I see, the element is created behind the curtains and replaced in the DOM once it's ready. Nice technique. 👍
|
@IvanSanchez just realized that probably I should create MR to 'v1' branch... or you care about cherry-picks yourself? |
Hi, |
|
@Falke-Design @IvanSanchez is there a change to merge this PR this year? Also, do you already have a plan regarding v2 release? If no, I can create also PR to v1 branch. |
Hi, please consider the possibility to merge this PR.
Background
We are happy to use Leaflet in our Karelics.Cloud platform. One problem we faced with is map tiles update. If map was changed and client wants to update tiles the only available GridLayer method is 'redraw' - it deletes all tiles and creates them back. Blinking caused by tiles removal looks bad in case updates happen often.
What was done
In general, GridLayer logic was not touched. Just piece of code was extracted from _addTile into _initTileElement method to be reused.
Only new thing is method reloadTiles. It creates new HTML elements for each existing tile and replaces tile's HTML element with new one on success. There is no tile removal, so visually this way to update tiles looks much better.
Give me know in case some edge case was not considered. Thanks!