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 duplicate entries in nav.html and default.html #239

Merged
merged 6 commits into from
Apr 28, 2020

Conversation

KasparEtter
Copy link
Contributor

This commit resolves #207.

Removes children from the table of contents that
belong to a different page with the same title.
@KasparEtter KasparEtter changed the title Fix duplicate entries in nav.html Fix duplicate entries in nav.html and default.html Oct 18, 2019
@pmarsceill pmarsceill added the next-minor-release next minor release bump label Apr 27, 2020
@pmarsceill pmarsceill changed the base branch from master to v0.2.9 April 27, 2020 17:21
@pmarsceill
Copy link
Collaborator

@pdmosses could you take a look at this? Does this make sense to you?

@pdmosses
Copy link
Contributor

@pmarsceill I'll take a look and let you know

@pdmosses
Copy link
Contributor

@pmarsceill I added test files (locally) for the example shown in #207 and it gives the correct navigation hierarchy, using the grand_parent titles to avoid duplicated displays.

In fact I'd experienced the bug in a previous project of my own, but circumvented it by making all the titles unique.

Note that the recursive navigation proposal #192 doesn't involve grand_parent settings, so duplicate ancestor titles might be more difficult to support with that.

@pdmosses
Copy link
Contributor

@pmarsceill oops, the entries of the automatic TOCs for children are missing! For example, select UI Components. I'll take a look at the code in default.html.

@pdmosses
Copy link
Contributor

@pmarsceill I've found what I think is the bug in default.html. It refers to children_list, which is assigned in nav.html. Copying the following code from nav.html to default.html fixes it:

          {%- assign ordered_pages_list = site.html_pages | where_exp:"item", "item.nav_order != nil" -%}
          {%- assign unordered_pages_list = site.html_pages | where_exp:"item", "item.nav_order == nil" -%}
          {%- if site.nav_sort == 'case_sensitive' -%}
            {%- assign sorted_ordered_pages_list = ordered_pages_list | sort:"nav_order" -%}
            {%- assign sorted_unordered_pages_list = unordered_pages_list | sort:"title" -%}
          {%- else -%}
            {%- assign sorted_ordered_pages_list = ordered_pages_list | sort_natural:"nav_order" -%}
            {%- assign sorted_unordered_pages_list = unordered_pages_list | sort_natural:"title" -%}
          {%- endif -%}
          {%- assign pages_list = sorted_ordered_pages_list | concat: sorted_unordered_pages_list -%}
          {%- assign children_list = pages_list | where: "parent", node.title -%}

Should I commit that change to KasparEtter's fork?

@pdmosses
Copy link
Contributor

@pmarsceill in fact only the last line above, assigning to children_list, needs to be added in default.html: the value of pages_list in nav.html is subsequently available in default.html.

@pmarsceill
Copy link
Collaborator

@pdmosses is this good to merge into 0.2.9 now?

@pdmosses
Copy link
Contributor

@pmarsceill looking at it now, I noticed that the list of grandchildren in the nav uses site.html_pages, which is unsorted.

I have a local test directory with grandchildren; I'll expand it to check whether I can make it get the order wrong, and let you know.

Changed `site.html_pages` to `pages_list`, to repeat the nav order of the grandchildren in the nav panel.
@pdmosses
Copy link
Contributor

My suspicion was justified: in my local test with grandchildren, the nav order was not respected. The update I committed appears to have corrected that.

@pdmosses
Copy link
Contributor

@pmarsceill good to merge into 0.2.9 now, I believe.

@pmarsceill pmarsceill merged commit 484563b into just-the-docs:v0.2.9 Apr 28, 2020
@pmarsceill pmarsceill mentioned this pull request Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-minor-release next minor release bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Markdown titles need namespaces
3 participants