Skip to content

Fixed issue 1936: brackets sorting#2719

Merged
anth-volk merged 1 commit into
masterfrom
issue-1936-brackets-not-in-chronological-order
Aug 13, 2025
Merged

Fixed issue 1936: brackets sorting#2719
anth-volk merged 1 commit into
masterfrom
issue-1936-brackets-not-in-chronological-order

Conversation

@AnushkaNilangekar
Copy link
Copy Markdown
Collaborator

Fixes #1936

Description

This PR fixes the incorrect ordering of bracketed parameter nodes (e.g., "Bracket 10" appearing before "Bracket 2") in the parameter tree sidebar. The issue was caused by missing index assignments for bracket nodes in the parameter tree construction, and by sorting logic that did not distinguish between bracket and non-bracket nodes. This change ensures bracket nodes are always sorted numerically, while all other nodes are sorted alphabetically.

Changes

Parameter Tree Construction (parameters.js)

Assigns a numeric index to bracket nodes during tree construction, based on their bracket number.
Non-bracket nodes retain their default or module-based index.

Tree Sorting Logic (PolicyPage.jsx):

Updates the sortTreeInPlace function to:
Detect if all nodes at a given tree level are bracket nodes (using a regex on the label).
Sort bracket nodes numerically by their index property.
Sort all other nodes alphabetically by their label.

Screenshots

https://www.loom.com/share/cec71fa84dae43e6b58b7e08b49782d1?sid=3d6c2f5a-b909-4bdf-9dbd-790bf173b26d

Tests

Manually verified in the UI that bracketed parameters are now always sorted numerically, even for double-digit brackets. Confirmed that non-bracket nodes remain sorted alphabetically.

@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
policyengine-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 5, 2025 1:34am

Copy link
Copy Markdown
Collaborator

@anth-volk anth-volk left a comment

Choose a reason for hiding this comment

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

Thanks for these contributions @AnushkaNilangekar! They're a real improvement to the UI. Generally, we'd ask for tests to accompany these, but given that we will be looking to deprecate this module by the end of the year, as well as the narrow scope of the changes, we will skip that this time.

@anth-volk anth-volk merged commit 50f9a70 into master Aug 13, 2025
4 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in policyengine-app Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Brackets do not appear in chronological order if more than 10

2 participants