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

Make crumbs available in all pages #3045

Merged
merged 2 commits into from Apr 15, 2018
Merged

Make crumbs available in all pages #3045

merged 2 commits into from Apr 15, 2018

Conversation

@tbm
Copy link
Contributor

@tbm tbm commented Apr 15, 2018

Make the crumbs variable available in all pages so templates can
have access to it.

@Kwpolska suggested I create a plugin to have breadcrumbs for pages but I don't see any reason not to make crumbs available all the time so templates can have access to it.

nikola/nikola.py Outdated
@@ -2183,6 +2183,7 @@ def generic_page_renderer(self, lang, post, filters, context=None):
context['title'] = post.title(lang)
context['description'] = post.description(lang)
context['permalink'] = post.permalink(lang)
context['crumbs'] = utils.get_crumbs(post.destination_path(lang, extension))
Copy link
Member

@Kwpolska Kwpolska Apr 15, 2018

This may override another crumbs variable in the context. Perhaps do if 'crumbs' not in context?

@tbm
Copy link
Contributor Author

@tbm tbm commented Apr 15, 2018

I just noticed another issue. I'll post a new version soon.

@tbm tbm force-pushed the crumbs-pages branch from 12fd63d to a9a0271 Apr 15, 2018
@tbm
Copy link
Contributor Author

@tbm tbm commented Apr 15, 2018

I think the new version works correctly regardless of whether PRETTY_URLS is true or false. At least it worked in my tests.

@tbm tbm force-pushed the crumbs-pages branch from a9a0271 to e71fb52 Apr 15, 2018
tbm added a commit to tbm/homepage that referenced this issue Apr 15, 2018
Add breadcrumbs, including pretty breadcrumbs (crumbs defined in
metadata instead of filepaths).

This relies on the following change to Nikola which has not been
acceptd yet: getnikola/nikola#3045
tbm added a commit to tbm/homepage that referenced this issue Apr 15, 2018
Add breadcrumbs, including pretty breadcrumbs (crumbs defined in
metadata instead of filepaths).

This relies on the following change to Nikola which has not been
acceptd yet: getnikola/nikola#3045
nikola/nikola.py Outdated
@@ -2183,6 +2183,11 @@ def generic_page_renderer(self, lang, post, filters, context=None):
context['title'] = post.title(lang)
context['description'] = post.description(lang)
context['permalink'] = post.permalink(lang)
if 'crumbs' not in context:
if post.permalink(lang).endswith('/'):
Copy link
Member

@Kwpolska Kwpolska Apr 15, 2018

How does this work if STRIP_INDEXES = False?

Copy link
Contributor Author

@tbm tbm Apr 15, 2018

If it's False', it's a file (which probably ends with .html) and files are dealt with in the else clause.

So for posts/foo.html we'd end up with

context['crumbs'] = [['.', 'posts'], ['#', 'foo.html']]

I've also tested `foo/bar/' which leads to

context['crumbs'] = [['..', 'foo'], ['#', 'bar']]

Hmm, I guess PRETTY_URLS = True and STRIP_INDEXES = False is not handled ideally (although not wrong either). foo/bar/index.html would be:

context['crumbs'] = [['..', 'foo'], ['.', 'bar'], ['#', 'index.html']]

Is this your concern? Or did I miss some other case?

Copy link
Member

@Kwpolska Kwpolska Apr 15, 2018

Yeah, that last one is kinda concerning (although this use case is a strange one). You can do permalink.endswith(self.config['INDEX_FILE']) and then do some extra logic.

Make the crumbs variable available to all pages so templates can
have access to it.
@tbm tbm force-pushed the crumbs-pages branch from e71fb52 to e3a9c15 Apr 15, 2018
@tbm
Copy link
Contributor Author

@tbm tbm commented Apr 15, 2018

This covers all combinations I can think of.

nikola/nikola.py Outdated
@@ -2183,6 +2183,12 @@ def generic_page_renderer(self, lang, post, filters, context=None):
context['title'] = post.title(lang)
context['description'] = post.description(lang)
context['permalink'] = post.permalink(lang)
if 'crumbs' not in context:
crumb_path = post.permalink(lang).lstrip('/').rstrip(self.config['INDEX_FILE'])
Copy link
Member

@Kwpolska Kwpolska Apr 15, 2018

rstrip doesn’t do what you think it does. The string is used as a list of characters to find and remove, not as a sequence. You need to remove it somehow else:

crumb_path = post.permalink(lang).lstrip('/')
if crumb_path.endswith(self.config['INDEX_FILE']):
    crumb_path = crumb_path[:-len(self.config['INDEX_FILE'])]

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska Kwpolska merged commit 80bff39 into getnikola:master Apr 15, 2018
2 checks passed
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Apr 15, 2018

Thanks for contributing this! 🎉

@tbm
Copy link
Contributor Author

@tbm tbm commented Apr 16, 2018

Thank you.

Any chance of getting this into v7 too? I know it's more of a feature but you could argue not exporting the crumbs to posts is a bug ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants