MapEventData#isSourceLoaded fix #4254
Conversation
src/util/evented.js
Outdated
| // determine if source is loaded when event is fired | ||
| if (typeof data.isSourceLoaded === "function") { | ||
| data.isSourceLoaded = data.isSourceLoaded(); | ||
| }; |
There was a problem hiding this comment.
An alternative to this approach might be to change the semantics of Evented#setEventedParent() so that, if the evented parent data parameter is a function, it is evaluated during fire() rather than during setEventedParent().
On the surface, that seems preferable to me because it avoids the hardcoding here... but it would require a review of setEventedParent to make sure we aren't counting on data being evaluated immediately anywhere else.
mollymerp
force-pushed
the
event-chores
branch
from
a1d941a to
25f4742
Compare
|
to keep the scope of this PR under control, I suggest we ship this fix as-in and I will handle a larger rejiggering of the source |
| @@ -82,7 +81,7 @@ class Evented { | |||
| } | |||
|
|
|||
| if (this._eventedParent) { | |||
| this._eventedParent.fire(type, util.extend({}, data, this._eventedParentData)); | |||
| this._eventedParent.fire(type, util.extend({}, data, typeof this._eventedParentData === 'function' ? this._eventedParentData() : this._eventedParentData)); | |||
There was a problem hiding this comment.
Could you add a test that ensures the eventedParentData function is called per emit and passes on the latest & greatest data?
There was a problem hiding this comment.
Great -- added a test!
mollymerp
force-pushed
the
event-chores
branch
from
6f71c8d to
820f37f
Compare
Working on fixing evaluation of
MapEventData#isSourceLoadedFixes #3958
Questions:
We only fire a
dataevent withdataType: sourcewhen the TileJSON is loaded, butisSourceLoaded: trueonly occurs after all the tiles in for the source in the current viewport are loaded, so fordataType:sourceEventData#isSourceLoadedis still nevertrue. Should we fire anotherdataevent of typesourcewhen all tiles are loadedisSourceLoadedflips totrue?cc @lucaswoj @anandthakker @lbud
Launch Checklist