Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for DataLocationProxy returning null #40

Closed
wants to merge 1 commit into from
Closed

Fix for DataLocationProxy returning null #40

wants to merge 1 commit into from

Conversation

crofty
Copy link
Contributor

@crofty crofty commented Oct 1, 2010

Hi Aaron,
I ran into an issue when using the DataLocationProxy. The following app would fail with 'path is null'

var app = $.sammy('div.app',function(a){
a.setLocationProxy(new Sammy.DataLocationProxy(this));
a.get('#/',function(){
this.swap('Hello world')
})
});
app.run('#/');

This is because the jQuery data method returns null when there is no data set. The Sammy code expects an empty string to be returned when there is no location set as this is what is returned by the HashChangeProxy. I've updated the DataLocationProxy to return the same as the HashChangeProxy.

Would this be your preferred way of doing things, or should the sammy code (#823 - #824) check for an empty string and null rather than just an empty string?

Cheers
James

Changed DataLocationProxy to work like HashChangeProxy so that it
returns an empty string rather than null when the current location
has not been set.  This fixes the issue that causes the app below to
fail with a 'path is null' error.

var app = $.sammy('div.app',function(a){
  a.setLocationProxy(new Sammy.DataLocationProxy(this));
  a.get('#/',function(){
    this.swap('Hello world')
  })
});
app.run('#/');
@crofty
Copy link
Contributor Author

crofty commented Oct 1, 2010

Bloody formatting!. App code should read:

var app = $.sammy('div.app',function(a){
  a.setLocationProxy(new Sammy.DataLocationProxy(this));
  a.get('#/',function(){
    this.swap('Hello world')  
  })
});
app.run('#/');

@quirkey
Copy link
Owner

quirkey commented Oct 2, 2010

This looks right, going to pull it :) Thanks!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants