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

Add 'child_nav_order' front matter to be able to sort navigation pages in reverse #726

Merged
merged 9 commits into from Jul 4, 2022

Conversation

jmertic
Copy link
Contributor

@jmertic jmertic commented Oct 19, 2021

Set child_nav_order to desc to reverse the sort order for a child section.

Signed-off-by: John Mertic jmertic@linuxfoundation.org

…s in reverse

Set `child_nav_order` to `desc` to reverse the sort order for a child section.

Signed-off-by: John Mertic <jmertic@linuxfoundation.org>
@mattxwang
Copy link
Member

Hi @jmertic, thanks for your contribution; we're just now getting around to triaging all the open PRs.

Could you add a couple of lines of documentation explaining this new feature (I imagine in the Navigation Structure page)? Want to make sure that others know how to use your new feature!

Signed-off-by: John Mertic <jmertic@linuxfoundation.org>
@jmertic
Copy link
Contributor Author

jmertic commented Mar 10, 2022

Awesome - take a look at my last commit and see if that is the right place/tone/verbiage.

Copy link
Member

@mattxwang mattxwang left a comment

Choose a reason for hiding this comment

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

Hi @jmertic, thanks for the quick response (and sorry for my late one - been swamped with finals).

I think the current location of the docs is slightly misplaced - it seems like it's interrupting the "Pages with children" documentation.

My suggestion - could we instead add this as a subheading under "Pages with children" (maybe called "Ordering child pages"), and then include an example? Hopefully that shouldn't be too much work - let me know how I can help!

Signed-off-by: John Mertic <jmertic@linuxfoundation.org>
@jmertic
Copy link
Contributor Author

jmertic commented Mar 21, 2022

Thanks @mattxwang - I think I didn't what you were asking, let me know if that looks better.

Copy link
Member

@mattxwang mattxwang left a comment

Choose a reason for hiding this comment

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

Thanks @jmertic - left two minor suggestions, but otherwise LGTM! Feel free to commit the suggestions or provide your own.

docs/navigation-structure.md Outdated Show resolved Hide resolved
docs/navigation-structure.md Show resolved Hide resolved
jmertic and others added 2 commits March 23, 2022 16:28
Signed-off-by: John Mertic <jmertic@linuxfoundation.org>

Co-authored-by: Matt Wang <matt@matthewwang.me>
Signed-off-by: John Mertic <jmertic@linuxfoundation.org>

Co-authored-by: Matt Wang <matt@matthewwang.me>
@jmertic
Copy link
Contributor Author

jmertic commented Mar 23, 2022

Thank you @mattxwang - accepted these great suggestions.

Copy link
Member

@mattxwang mattxwang left a comment

Choose a reason for hiding this comment

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

Looks good to me! I think we should bundle this in to #779 then. Thank you for your contribution!

@mattxwang mattxwang changed the base branch from main to v0.4.0 March 23, 2022 20:38
@mattxwang mattxwang merged commit 6287d8a into just-the-docs:v0.4.0 Jul 4, 2022
@pdmosses
Copy link
Contributor

@mattxwang @jmertic What was the motivation for choosing desc for the value to activate this feature? Apparently "desc." abbreviates "descendant", rather than "descending".

Perhaps reversed would be a more appropriate word (related to the reversed attribute on ordered lists in HTML).

Moreover, the current implementation is:

          {%- if node.child_nav_order == 'desc' -%}
            {%- assign children_list = children_list | reverse -%}
          {%- endif -%}

I realise this is a late suggestion, but if we're ever going to make a breaking change to this feature, v0.4.0 would be the best opportunity… Issue #827 reminded me of this.

We could also support both desc and reversed, and deprecate desc to warn that it might disappear in a future release.

@jmertic
Copy link
Contributor Author

jmertic commented Dec 15, 2022

reversed works for me just as well - as you inferred the use of desc was for descending but I can understand the confusion.

Let me know if it's preferred to do the full switch, or support both but have desc as deprecated.

@pdmosses
Copy link
Contributor

Thanks for the immediate – and positive – response!

Let's see what @mattxwang prefers.

@mattxwang
Copy link
Member

Ah - still getting back into the swing of things.

  • no strong opinion on naming
  • if we do change, I think we should probably support both but deprecate desc - we can say that reversed is the preferred keyword, and deprecate it in v1.

Thoughts? Hopefully should be a simple change. Thanks for catching this @pdmosses and for the quick response @jmertic!

@jmertic
Copy link
Contributor Author

jmertic commented Dec 16, 2022

Created #1061 - let me know if this works.

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

3 participants