can.route triggers event two times for setting nested object. #269

Closed
schovi opened this Issue Feb 7, 2013 · 8 comments

Comments

Projects
None yet
4 participants
@schovi
Contributor

schovi commented Feb 7, 2013

I want to use two level object in route for control some panel with widgets.
When i want to make widget visible i can easily trigger
can.route.attr({notifications: {messages: true}})
and when i want to hide it i will call
can.route.removeAttr('notifications.messages')
and each widget listen on route change for given 'code'
can.route.bind('notifications.messages', function() {....})
Idea worked fine until i find some strange behaviour that event is triggered 2 times.
First for whole object {notifications ...}
Second for notifications.messages (And there it pass new value as stringified "true" and old value as standard true

Thing with stringified true is even more stranger :)

More in console in this Fiddle

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 8, 2013

Contributor

Does your pull request #272 solve this?

Contributor

daffl commented Feb 8, 2013

Does your pull request #272 solve this?

@schovi

This comment has been minimized.

Show comment
Hide comment
@schovi

schovi Feb 8, 2013

Contributor

No, it is different problem.

Contributor

schovi commented Feb 8, 2013

No, it is different problem.

@schovi

This comment has been minimized.

Show comment
Hide comment
@schovi

schovi Mar 18, 2013

Contributor

Maybe similar to this one #280

Contributor

schovi commented Mar 18, 2013

Maybe similar to this one #280

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Sep 20, 2013

Contributor

Part of the issue is:

can.route.attr({notifications: {messages: true}})

You are passing a non-string value, but this gets added to the hash, which converts it to a string, hence the double change.

We could try to force every incoming value to a string in can.route.

Can you create a test?

Contributor

justinbmeyer commented Sep 20, 2013

Part of the issue is:

can.route.attr({notifications: {messages: true}})

You are passing a non-string value, but this gets added to the hash, which converts it to a string, hence the double change.

We could try to force every incoming value to a string in can.route.

Can you create a test?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 11, 2015

Contributor

I think you can fix this with the define plugin:

var RouteState = can.Map.extend({
  define: {
    messages: {
       type: "boolean"
    }
  }
})

can.route.map(new RouteState);
can.route.ready()

can.route.bind("messages", function(){})

can.route.attr("messages", true);
can.route.attr("messages", false);
Contributor

justinbmeyer commented Feb 11, 2015

I think you can fix this with the define plugin:

var RouteState = can.Map.extend({
  define: {
    messages: {
       type: "boolean"
    }
  }
})

can.route.map(new RouteState);
can.route.ready()

can.route.bind("messages", function(){})

can.route.attr("messages", true);
can.route.attr("messages", false);
@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 11, 2015

Contributor

I would double check if this hasn't been fixed with #1187 yet which I merged yesterday.

Contributor

daffl commented Feb 11, 2015

I would double check if this hasn't been fixed with #1187 yet which I merged yesterday.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 11, 2015

Contributor

Nervemind, that looks like it's something different.

Contributor

daffl commented Feb 11, 2015

Nervemind, that looks like it's something different.

@daffl daffl added this to the 2.3.0 milestone Feb 12, 2015

@James0x57

This comment has been minimized.

Show comment
Hide comment
@James0x57

James0x57 Sep 17, 2015

Contributor

Looks to be fixed.
http://jsbin.com/remujuziho/edit?html,js,output

test( "deep param bind doesn't fire twice on remove", function () {
  expect( 1 );
  can.route.bind( "prop.deep", function () {
    ok( true, "only fired once because prop.deep changes" );
  });
  can.route.attr( { prop: { deep: true } } );
  // issue #269, remove would fire the change event for "prop.deep" AND "prop"
  can.route.removeAttr( "prop.deep" );
});
Contributor

James0x57 commented Sep 17, 2015

Looks to be fixed.
http://jsbin.com/remujuziho/edit?html,js,output

test( "deep param bind doesn't fire twice on remove", function () {
  expect( 1 );
  can.route.bind( "prop.deep", function () {
    ok( true, "only fired once because prop.deep changes" );
  });
  can.route.attr( { prop: { deep: true } } );
  // issue #269, remove would fire the change event for "prop.deep" AND "prop"
  can.route.removeAttr( "prop.deep" );
});

@James0x57 James0x57 closed this Sep 17, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment