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

chore: Update IndexedNode to always have a parentId property #462

Merged
merged 3 commits into from
Jul 29, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Jul 22, 2024

What?

  • Adds a required parentId property to IndexedNode
  • Updates sortFlow() logic to match PlanX graph structure

Why?

  • Implemented in feat: Initial frontend search proof of concept with Fuse.js planx-new#3435
  • For navigation to a node to work from search, we need to know the "path" through the graph to a node
  • Currently sortFlow() returns an optional parentId, despite all nodes having a parent (apart from the root node). It's more accurate to give all nodes a parent. This makes things a bit easier when constructing a path (upcoming PR)
  • The current implementation of sortFlow() doesn't quite match our graph structure. When reaching the next child of the root node, the current parentId is maintained instead of falling back to the root. This worked well for treating the graph as a linear structure to split it up be portal names (pseudo sections), but doesn't work in terms of actually describing the hierarchy of the graph (required to build a valid PlanX URL).
  • The difference really comes down to "previous node" vs. "parent node"
  • The flow will still retain the same sorted order, root.edges[0]root.edges[1]root.edges[2] → etc

Note: This should not impact any current functionality. sortFlow() and findNextNodeOfType() are not used anywhere in PlanX currently.

Example

  • I have a root node with two edges
  • I navigate through the branches of the first edges until there are no more edges
  • My next step will be the second edge of the root node
  • Currently: The parentId for root.edges[1] will be set to the very final node of root.edges[0] (previous node)
  • This PR: The parentId for root.edges[1] will be set to "_root" (parent node)

@@ -107,41 +107,6 @@ export function sortBreadcrumbs(
return orderedBreadcrumbs;
}

export function findNextNodeOfType({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function (and associated tests) has been removed as it's not used, and doesn't work as expected.

@DafyddLlyr DafyddLlyr requested a review from a team July 26, 2024 07:33
@DafyddLlyr DafyddLlyr marked this pull request as ready for review July 26, 2024 07:41
@DafyddLlyr DafyddLlyr force-pushed the dp/update-flow-types branch from 8b1c870 to 339cb50 Compare July 29, 2024 10:20
@DafyddLlyr DafyddLlyr merged commit 4698fc2 into main Jul 29, 2024
3 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/update-flow-types branch July 29, 2024 11:16
DafyddLlyr added a commit that referenced this pull request Jul 29, 2024
## What does this PR do?
- Generates a path for a node, which is required for search
- Relies on #462
- Implemented in
theopensystemslab/planx-new#3460
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.

2 participants