feat: [LC-1442][LC-1522][LC-1484][LC-1465] Self-Assigned Skills + other skill UI updates#1006
feat: [LC-1442][LC-1522][LC-1484][LC-1465] Self-Assigned Skills + other skill UI updates#1006
Conversation
There was a problem hiding this comment.
β¨ PR Review
The PR implements a comprehensive self-assigned skills feature with proficiency levels. The implementation is generally well-structured, but there is a critical logic error in the error detection for framework loading.
1 issues detected:
π Bug - The boolean logic is inverted - it checks for loading=true instead of loading=false for the error condition π οΈ
Details: The errorLoadingFramework condition incorrectly treats the loading state as an error state. The current logic
!selfAssignedSkillFramework && selfAssignedSkillFrameworkLoadingevaluates to true WHILE the framework is still loading, which will incorrectly display an error message and disable UI buttons during the loading phase. The error state should only be true when loading is complete but no framework was returned.
File:apps/learn-card-app/src/pages/skills/SelfAssignSkillsModal.tsx (239-239)
π οΈ 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
|
@rhen92 Aha! Skill tiers were showing up in the modal, but those don't get displayed in Skills Hub because they're not actually skills, they're just containers. Updated semantic search to exclude tiers and added a safeguard so that SelfAssignedSkilsRow just doesn't display if it's a tier in case anything gets through (e.g. through initialSkillIds) |
There was a problem hiding this comment.
β¨ PR Review
The PR implements a comprehensive self-assigned skills feature with proficiency levels. The code is generally well-structured, but there's a critical bug in the proficiency level synchronization logic that will prevent external updates to the "Hidden" proficiency level from being applied.
1 issues detected:
π Bug - The falsy check treats 0 (SkillLevel.Hidden) as false, preventing synchronization when proficiency level is set to Hidden π οΈ
Details: The effect that syncs external proficiencyLevel changes uses a falsy check (if (proficiencyLevel && ...)) which fails when proficiencyLevel is 0 (SkillLevel.Hidden). This prevents external updates to the Hidden proficiency level from being applied to the component's internal state, breaking synchronization for that specific value.
File:apps/learn-card-app/src/pages/skills/SkillProficiencyBar.tsx (203-203)
π οΈ 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
Co-authored-by: gitstream-cm[bot] <111687743+gitstream-cm[bot]@users.noreply.github.com>
There was a problem hiding this comment.
β¨ PR Review
The PR implements a comprehensive self-assigned skills feature with proficiency levels. The code is generally well-structured, but there's a minor issue with the error state condition in the SelfAssignSkillsModal that could show a misleading error during initial flag loading.
1 issues detected:
π Bug - Error state is triggered before the query runs, showing misleading error during initialization π οΈ
Details: The errorLoadingFramework condition evaluates to true before the framework query runs, causing a misleading error message to appear briefly during initial flag loading or when the frameworkId flag is not set. The condition checks
!selfAssignedSkillFramework && !selfAssignedSkillFrameworkLoadingbut doesn't account for when the query is disabled (frameworkId is undefined). This results in showing "Error loading framework" even though no loading attempt was made.
File:apps/learn-card-app/src/pages/skills/SelfAssignSkillsModal.tsx (239-239)
π οΈ 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
|
π₯· Code experts: TaylorBeeston TaylorBeeston has most π©βπ» activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: β¨ Comment |
Overview
π Relevant Jira Issues
LC-1442
LC-1522
LC-1484
LC-1465
π What is the context and goal of this PR?
Give users the ability to assign themselves skills from a specific framework. They should be able to click the plus button from the Skills page, select skills they desire, query for skills in the framework with semantic search, include a proficiency level if they so choose, click save, and then those skills should show up on their skills hub. When opening the skill info a special skill card should show up under "issuances" for the self-assigned skills. If the proficiency level was set to anything other than "hidden" it will be displayed on the skill info modal as well.
This PR also updates that skill info modal. Specifically, it adds tabs for the skill info + issuances and puts the framework info + related skills (parent, siblings, and similar) under a Details tab.
Also, when selecting skills, at the bottom of the search results there is a button to suggest a skill (i.e. your search query), which adds a row of data to this Google Sheet (needs an LEF account to view)
π₯΄ TL; RL:
Self-Assigned Skills! And skill UI updates.
π‘ Feature Breakdown (screenshots & videos encouraged!)
https://www.loom.com/share/4d10ba31b45b47a29a7c827a1e7f2eeb
π Important tradeoffs made:
π Types of Changes
π³ Does This Create Any New Technical Debt? ( If yes, please describe and add JIRA TODOs )
Testing
π¬ How Can Someone QA This?
SKILL_EMBEDDING_GOOGLE_API_KEYincompose-local.yamlSKILL_EMBEDDING_GOOGLE_API_KEY: ${SKILL_EMBEDDING_GOOGLE_API_KEY:-secretSecretHere}wef_global_skills_taxonomy_edited.json
initialSkillIdsif you don't want to deal with LDπ± π₯ Which devices would you like help testing on?
π§ͺ Code Coverage
Documentation
π Documentation Checklist
User-Facing Docs (
docs/β docs.learncard.com)docs/tutorials/)docs/how-to-guides/)docs/sdks/)docs/core-concepts/)docs/apps/)Internal/AI Docs
Visual Documentation
π Documentation Notes
β PR Checklist
π Ready to squash-and-merge?:
β¨ PR Description
Purpose: Add self-assigned skills feature to Skills Hub enabling users to self-attest skill proficiency levels stored on boost-skill relationships
Main changes:
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