question

tarasvozniuk avatar image
0 Votes"
tarasvozniuk asked tarasvozniuk commented

Bug report: Traffic control option isActive doesn't work when control is added in map ready handler

Description


The Traffic control options allows specifying isActive which should enable the traffic layers by default and set the control in enabled state (if map hasn't enabled traffic before with setTraffic).

However, when the traffic control is added inside map ready event callback:

 map.events.add('ready', function () { 
   //Create a traffic control to let the user easily turn the traffic on an off. 
   map.controls.add(new atlas.control.TrafficControl({isActive: true, style: 'dark'}), { 
     position: 'top-right'
   }); 
 }); 

the control gets toggled on and then off thus resulting in traffic not being displayed and control not active.

Expected Behavior


isActive: true should properly set control in enabled state and have the traffic displayed.

How to reproduce


In Azure Maps Traffic control codepan from official docs: https://codepen.io/azuremaps/pen/ZEWaeLJ just add isActive: true to traffic control init options

Explaination


The issue happens due to TrafficControl registering a map ready event callback itself:

 TrafficControl.prototype.constructTrafficButton = function (map) { 
         var _this = this; 
     
         var trafficButton = document.createElement("button"); 
         // some details removed out 
     
         trafficButton.addEventListener("click", function () { 
           var t = map.getTraffic(); 
           _this.options.isActive = !(t.flow !== "none" || t.incidents); 
     
           if (_this.options.isActive) { 
             map.setTraffic({ 
               flow: _this.options.flow, 
               incidents: _this.options.incidents 
             }); 
     
             _this.container.classList.add("in-use"); 
           } else { 
             map.setTraffic({ 
               flow: "none", 
               incidents: false 
             }); 
     
             _this.container.classList.remove("in-use"); 
           } 
         }); 
         map.events.add("ready", function () { 
           if (_this.options.isActive) { 
             trafficButton.dispatchEvent(new Event("click")); 
           } 
         }); 
         return trafficButton; 
       }; 

So the ready event handler gets added in another ready event handler, causing the callbacks that are getting iterated upon mutated during the iteration in:

 EventManager.prototype._invokeListeners = function (eventType, layer, args) { 
         var _this = this; 
     
         var callbacks = this.mapCallbackHandler.getEventCallbacks(eventType, layer); 
     
         if (callbacks) { 
           // event callbacks are mutated during the iteration 
           callbacks.forEach(function (_a, callback) { //... }) 
         } 
         // details redacted 
 } 

which results in TrafficControl ready handler triggered twice and thus resulting in enabled state toggled on and off.

Possible resolution


  1. Iterate over a copy of callback in EventManager._invokeListeners: something like (new Map(callbacks)).forEach()

  2. Reconsider the logic of TrafficControl initialization: ready control handle fires map click event that derives isActive state based on flipping the current traffic state (this can lead to unexpected behavior for the user if the map.setTraffic({incidents: true, flow: 'relative'}) was called and the TrafficControl was created with isActive, which results in traffic getting toggled off at init)



azure-maps
5 |1600 characters needed characters left characters exceeded

Up to 10 attachments (including images) can be used with a maximum of 3.0 MiB each and 30.0 MiB total.

IoTGirl avatar image
0 Votes"
IoTGirl answered

Hi anonymous user,

I have placed this issue on the Azure Maps SDK backlog for review but I believe this issue is easily mitigated by using the "setTraffic" option correct? How is your solution impacted by this issue? An explanation of the severity would help us understand the priority to give this issue.

Sincerely,
IoTGirl

5 |1600 characters needed characters left characters exceeded

Up to 10 attachments (including images) can be used with a maximum of 3.0 MiB each and 30.0 MiB total.

tarasvozniuk avatar image
0 Votes"
tarasvozniuk answered tarasvozniuk commented

I have placed this issue on the Azure Maps SDK backlog for review

@IoTGirl: Thanks!

I have placed this issue on the Azure Maps SDK backlog for review but I believe this issue is easily mitigated by using the "setTraffic" option correct?

You mean avoiding the traffic control and using directly map.setTraffic({incidents: true, flow: 'relative'})? The issue can be mitigated by adding the traffic control inside setTimeout(() =>{}) inside ready handler just fine.

How is your solution impacted by this issue?

Not much, I noticed it while testing the control abstraction for react at WiredSolutions/react-azure-maps/pull/91 , specifically in my solution, I just patched your atlas.js bundle directly. I am basically just reporting it here so issue is known. (since I haven't found any dedicated bug tracker, hopefully this is the right place to report these)



· 2
5 |1600 characters needed characters left characters exceeded

Up to 10 attachments (including images) can be used with a maximum of 3.0 MiB each and 30.0 MiB total.

anonymous user
Yep! This is the right place! I have added the details you provided and a link to this MS Q&A item so if you have more details to add later, you can place them here and the team should click through and see them.

0 Votes 0 ·

awesome, thanks!

0 Votes 0 ·
rbrundritt avatar image
1 Vote"
rbrundritt answered tarasvozniuk commented

Another option that doesn't require setTimeout is to use the load event instead of ready.

map.events.add('load', function () {

    map.controls.add(new atlas.control.TrafficControl({
        isActive: true
    }), {
        position: 'top-right'
    });
});


Also, in your original code block you added the traffic control options to the control add options which is different. Be sure to pass your traffic control options into the traffic control.

· 1
5 |1600 characters needed characters left characters exceeded

Up to 10 attachments (including images) can be used with a maximum of 3.0 MiB each and 30.0 MiB total.

Thanks for pointing out, load event is cleaner.

Oh yes, I've modified the question with the options in the the right place, thanks.

0 Votes 0 ·