Repository navigation
Conversation
Fixes the issue where sidebar navigation items lost active highlight when navigating from section list pages to nested detail pages (e.g. Tasks and Runs). Supports nestedPaths for Tasks, boundary-safe prefix matching, and preserves dashboard and favorite isolation.
|
|
Caution Review failedThe pull request is closed.
Tip 🎯 Customize your risk assessments
✅ Automatic approvals, with your rules
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
Hi @Ashik9500, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| nestedPaths={[ | ||
| `${v3EnvironmentPath(organization, project, environment)}/tasks`, | ||
| `${v3EnvironmentPath(organization, project, environment)}/agents`, | ||
| ]} |
There was a problem hiding this comment.
🟡 Sidebar fix missing release note
This user-facing webapp fix has no .server-changes/ entry. The release summary omits the sidebar highlight fix.
Learn more
The sidebar change fixes a visible behavior for webapp users. Server-only, user-facing fixes require a release note under .server-changes/, and this PR changes only webapp files. The release process includes those notes in its summary.
Example: A user upgrades and reads the release summary to see whether nested task pages retain their sidebar highlight. With no entry, the fix is absent from that summary.
Recommended fix: Add a .server-changes/ Markdown file with area: webapp, type: fix, and one user-facing sentence about retaining the sidebar highlight on detail pages.
Was this helpful? React with 👍 or 👎 to provide feedback.
| normalizePath, | ||
| } from "~/components/navigation/SideMenuItem"; | ||
|
|
||
| describe("Sidebar Navigation Route Matching", () => { |
There was a problem hiding this comment.
🟡 Unit tests lack required exception comment
The new sideMenuNavigation unit suite omits the required one-line comment explaining its exception to the integration-test preference.
Learn more
Repository test instructions allow new unit tests only exceptionally, for complex, stable logic unlikely to change, and require a one-line comment explaining the exception. This suite tests the isolated route-matching helper, but begins without that explanation.
Example: A reviewer opening the suite sees hundreds of helper assertions without a reason for choosing unit tests over integration coverage.
Recommended fix: If this matching logic qualifies for the exception, add a concise one-line comment explaining why. Otherwise test sidebar behavior through an integration path instead.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes the issue where sidebar navigation items lost active highlight when navigating from section list pages to nested detail pages (e.g. Tasks and Runs). Supports nestedPaths for Tasks, boundary-safe prefix matching, and preserves dashboard and favorite isolation.
Closes #
✅ Checklist
Testing
[Describe the steps you took to test this change]
Changelog
[Short description of what has changed]
Screenshots
[Screenshots]
💯