-
Notifications
You must be signed in to change notification settings - Fork 10
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
Sort NavigationTree
alphabetically
#112
Changes from 3 commits
2483e74
28f0d69
344f491
5ea8f66
d62e01d
1bb7a01
fce23a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
--- | ||
title: 'Accordion' | ||
description: 'UI component named after a funky instrument 🪗' | ||
path: 'Content/Accordion' | ||
--- | ||
import {Accordion} from '.' | ||
|
||
<Accordion/> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import { ReactElement } from 'react' | ||
|
||
interface AccordionProps { | ||
color?: string | ||
children: ReactElement | ||
} | ||
|
||
export function Accordion({ children }: AccordionProps) { | ||
return ( | ||
<details> | ||
<summary>Hello 👋</summary> | ||
Nice meeting you! How's everything going? | ||
</details> | ||
) | ||
} | ||
Comment on lines
+8
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the built in HTML elements for this, I imagine down the line we'll use the components package Dylan is working on |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import { | |
ComponentNode, | ||
CategoryNode, | ||
NavigationTree, | ||
NavigationNode, | ||
} from './types.js' | ||
|
||
export function getNavigationTree( | ||
|
@@ -83,5 +84,29 @@ export function getNavigationTree( | |
} | ||
|
||
const tree = Array.from(categories.values()) | ||
|
||
tree.sort(compareTitleSort) | ||
|
||
return tree | ||
} | ||
|
||
function compareTitleSort<T extends CategoryNode | NavigationNode>( | ||
a: T, | ||
b: T | ||
): -1 | 0 | 1 { | ||
const aHasChildren = 'children' in a && a.children.length > 0 | ||
const bHasChildren = 'children' in b && b.children.length > 0 | ||
if (aHasChildren) { | ||
a.children.sort(compareTitleSort) | ||
} | ||
if (bHasChildren) { | ||
b.children.sort(compareTitleSort) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm thinking this might not be the proper place to call sort recursively, as each element might get passed to the compare function more than once, which would result in us sorting an element's children multiple times. We'll likely want to separate out traversing the tree from the actual sorting. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you suggesting, something like:
|
||
if (a.title > b.title) { | ||
return 1 | ||
} | ||
if (b.title > a.title) { | ||
return -1 | ||
} | ||
return 0 | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made the compare function recursive per our discussion. @BRKalow This way the tree will be sorted no matter how deep the structure gets as long as we keep using the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a simple accordion to show the sorting