Fix link pathing #257
Fix link pathing #257
Conversation
templates/title.mustache
Outdated
| <ul class="title-links"> | ||
| <!-- <li><a href="#">docco</a></li> --> | ||
| {{#with (sourceLink .)}}<li><a href="{{.}}">Edit on GitHub</a></li>{{/with}} | ||
| <li><a href="{{sourceLink (repoName)}}">Edit on GitHub</a></li> |
There was a problem hiding this comment.
Would {{#sourceLink(repoName)}} work outside the <li> to prevent the link from being rendered if we can’t get the link?
There was a problem hiding this comment.
No, I don't think it would. However I haven't been able to reproduce a situation where sourceLink fails to get a link.
That would only occur if docMapInfo.getCurrent().src is undefined and I haven't seen that. Do you know if that's a likely situation?
There was a problem hiding this comment.
I did a quick check on if docMapInfo.getCurrent().src === undefined occurs. It does, but apparently only when rendering subsection headers, for example, "behaviors" under the can-connect. That "virtual parent page" doesn't have content so it's just an ordering in the side menu and doesn't render a page where this tag without a link would be visible.
From what I can tell the removal of the {{#with}} shouldn't have any side effects.
There was a problem hiding this comment.
FYI, those group pages do exist: https://canjs.com/doc/can-connect.behaviors.html
I’m not sure why the edit button doesn’t render now (I think #with is always supposed to render), but it wouldn’t hurt to check if the changes in this PR affect that.
There was a problem hiding this comment.
For interests sake, is there any way to navigate to that page without knowing the URL? Should we add some content (an index of it's children?) to that page seeing as it exists?
For now I can change this back as removing the #with now causes the page to have an edit link.
There was a problem hiding this comment.
There will be with search: #259
In that issue I debate that we should either not have those pages at all. If we keep them, then yeah, better content would be in order.
chasenlehara
left a comment
chasenlehara
left a comment
There was a problem hiding this comment.
I am assuming this will also fix these issues: #204 canjs/can-define#176
|
Yeah this resolves #204 and canjs/can-define#176 also |
…nt when you can get the variables from the template.
|
Simplified the helpers so they get the input from the template instead of the docMapInfo, which solves the problem with the GitHub link appearing on pages like |
|
@imaustink Does this look good to you? |
|
Looks solid to me. |
Resolves #255.
Assumes the page parent repository is either the same as the NPM module name in the path to the page, or if the page is not inside node_modules, the repository is 'canjs'.