-
Notifications
You must be signed in to change notification settings - Fork 8
Feat: [LC-1411] Quick Nav #862
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
Conversation
… launchpad aciton modal
…ws are involved via query param
🦋 Changeset detectedLatest commit: e0e008d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for learncarddocs canceled.
|
✅ Deploy Preview for staging-learncardapp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…after opening boost cms
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.
✨ PR Review
The PR introduces a comprehensive Quick Nav feature with role-based actions and improved navigation. The implementation is generally solid, but there are several critical issues that need attention: a React hook dependency violation, a potential regex injection vulnerability, and unsafe type assertions that bypass TypeScript's type safety.
3 issues detected:
🐞 Bug - The effect reads `claim` but doesn't declare it as a dependency, causing potential stale closure bugs
Details: The useEffect hook that conditionally shows LaunchPadActionModal uses the
claimvariable in its logic (line 177-179) but does not include it in the dependency array. This violates React's exhaustive-deps rule and can lead to stale closures where the effect uses an outdated value ofclaimif it changes between renders.
File:apps/learn-card-app/src/pages/launchPad/LaunchPad.tsx🔒 Security - Unescaped user input in RegExp constructor can break search or cause unexpected matching 🛠️
Details: User input from
debouncedSearchInputis directly passed to the RegExp constructor without escaping special regex characters. Users could input characters like[,],(,),*,+, etc., which would either break the search functionality or cause unexpected matching behavior. While this is client-side, it can still cause the application to crash or behave unpredictably.
File:apps/learn-card-app/src/pages/launchPad/LaunchPadHeader/IssueManagedBoostSelector.tsx (100-114)
🛠️ A suggested code correction is included in the review comments.🧹 Maintainability - Using `as any` bypasses type safety and can hide runtime type errors
Details: The code uses
as anyto cast the result ofuseLaunchPadContracts()toLaunchPadItem[]. This completely bypasses TypeScript's type checking and can hide type mismatches that could cause runtime errors. The type assertion suggests a fundamental type incompatibility that should be properly resolved rather than suppressed.
File:apps/learn-card-app/src/pages/launchPad/LaunchPad.tsx
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
apps/learn-card-app/src/pages/launchPad/LaunchPadHeader/IssueManagedBoostSelector.tsx
Outdated
Show resolved
Hide resolved
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.
✨ PR Review
The PR introduces a comprehensive Quick Nav feature with role-based actions. The implementation is generally solid with good type safety improvements in helper functions. However, there's a dependency array issue in a useEffect that could cause stale closures.
1 issues detected:
🐞 Bug - The effect reads `claim` on line 177 but doesn't list it as a dependency, violating React's rules of hooks
Details: The useEffect hook uses the
claimquery parameter to determine whether to skip showing the action modal, butclaimis not included in the dependency array. This could cause the effect to use a stale value ofclaimif the query parameter changes.
File:apps/learn-card-app/src/pages/launchPad/LaunchPad.tsx
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
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.
✨ PR Review
The PR adds a comprehensive Quick Nav feature with role-based actions and improved navigation. The implementation is generally solid, but there are security concerns with user input handling that need to be addressed before merging.
1 issues detected:
🔒 Security - User input is used directly in RegExp constructor without sanitization, which can break search functionality or cause runtime errors. 🛠️
Details: User input from debouncedSearchInput is directly passed to the RegExp constructor without escaping special regex characters. This can cause the application to crash or behave unpredictably when users input special characters like '[', ']', '(', ')', '*', '+', etc.
File:apps/learn-card-app/src/pages/launchPad/LaunchPadHeader/IssueManagedBoostSelector.tsx (114-114)
🛠️ A suggested code correction is included in the review comments.
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
apps/learn-card-app/src/pages/launchPad/LaunchPadHeader/IssueManagedBoostSelector.tsx
Outdated
Show resolved
Hide resolved
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.
✨ PR Review
The PR adds a comprehensive Quick Nav feature with role-based actions. The implementation is generally well-structured, but there is a critical security issue with unescaped regex input in the new IssueManagedBoostSelector component that needs to be addressed.
1 issues detected:
🔒 Security - Unescaped user input in RegExp constructor can cause crashes or unexpected behavior 🛠️
Details: User input from debouncedSearchInput is passed directly to the RegExp constructor without escaping special regex characters. Users could input characters like '[', ']', '(', ')', '*', '+', etc., which would either break the search functionality or cause the application to crash with a regex parsing error.
File:apps/learn-card-app/src/pages/launchPad/LaunchPadHeader/IssueManagedBoostSelector.tsx (114-114)
🛠️ A suggested code correction is included in the review comments.
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
apps/learn-card-app/src/pages/launchPad/LaunchPadHeader/IssueManagedBoostSelector.tsx
Outdated
Show resolved
Hide resolved
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.
Screen.Recording.2025-12-18.at.2.58.44.PM.mov
@goblincore super slick! LGTM! 🚀🚀
Overview
Will add Loom explainer video later after I get some sleep but this is ready
🎟 Relevant Jira Issues
https://welibrary.atlassian.net/browse/LC-1411
📚 What is the context and goal of this PR?
Add Quick Nav feature on the Launchpad which is a new navigation component that allows a user to easily access certain actions related to a role they select including the new AI insights and skills management features
🥴 TL; RL:
New Quick Nav
💡 Feature Breakdown (screenshots & videos encouraged!)
See video for general idea:
Screen.Recording.2025-12-18.at.5.21.43.PM.mov
Updated Launchpad with trigger to new quick nav (also triggers automatically upon login though disabled if claiming and other flows):

QuickNav:

Role Selector:

New Issue Credentials Selector and Modal:

/ai/insights?tab=learner-insights/skills?tab=admin-panel/ai/insights?tab=child-insights/ai/insightsrespectstabquery param for initial tab selection./skillsrespectstabquery param for initial tab selection (My Hub / Admin Panel).classNamefor consistent sizing.🛠 Important tradeoffs made:
Does not attempt to refine components that were already made that are accessed from quick menu so that they better fit together. EG for example as guardian you can issue a boost to a child and it just brings up the existing child selector and doesn't make a new custom component that might be better UI
Used URL query params for tab pre-selection to keep navigation simple and shareable.
🔍 Types of Changes
💳 Does This Create Any New Technical Debt? ( If yes, please describe and add JIRA TODOs )
Testing
🔬 How Can Someone QA This?
Login and test it out for various roles.
📱 🖥 Which devices would you like help testing on?
🧪 Code Coverage
No new automated tests added (UI routing + modal wiring).
Documentation
📜 Gitbook
Not needed.
📊 Storybook
Not applicable.
✅ PR Checklist
🚀 Ready to squash-and-merge?:
✨ PR Description
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how