Skip to content

Conversation

@imaustink
Copy link
Contributor

No description provided.

@chasenlehara
Copy link
Member

This doesn’t seem to be working for me:
not working

@imaustink
Copy link
Contributor Author

@chasenlehara my bad! I clearly didn't test that good enough. Updated the PR 😄

static/search.js Outdated
var a = $a[0];
var isLocal = a.hostname === location.hostname;
var isRelative = a.getAttribute('href').indexOf(location.hostname) === -1;
if(a.hostname !== location.hostname || a.protocol !== location.protocol){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a.hostname !== location.hostname could be !isLocal

$a.attr('target', '_blank');
}
if(isLocal && isRelative){
var prefix = parentHref.substr(0, parentHref.lastIndexOf('/'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments on lines like this would make the code easier to read.


if(this.url.substr(-1) === "/"){
if(url.substr(-1) === "/"){
return this.url;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be just url?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static/search.js Outdated
var $html = $('<div>').html(html);
$html.find('a').each(function(){
var $a = $(this);
var a = $a[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going through this dance to convert the object to be jQuery-wrapped and then to unwrap it seems odd.

@imaustink imaustink merged commit 97765ea into master Jun 29, 2017
@imaustink imaustink deleted the fix-description-link-paths branch June 29, 2017 22:35
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.

3 participants