feat: Replace loading spinners with skeleton UI in mentorship portal#3814
Closed
fasiho7 wants to merge 1 commit intoOWASP:mainfrom
Closed
feat: Replace loading spinners with skeleton UI in mentorship portal#3814fasiho7 wants to merge 1 commit intoOWASP:mainfrom
fasiho7 wants to merge 1 commit intoOWASP:mainfrom
Conversation
Major UX improvement to enhance perceived loading time and user experience ## Features Added - Created comprehensive skeleton UI components for mentorship portal - ProgramCardSkeleton - Mimics program card structure with status badges, titles, descriptions - ModuleCardSkeleton - Mimics module card structure with drag handles, progress bars - MentorshipPageSkeleton - Complete page-level skeletons for different content types ## Technical Implementation - Added 3 new skeleton components with proper ARIA roles and accessibility - Enhanced SkeletonBase to support mentorship-specific skeletons - Replaced all LoadingSpinner instances in mentorship pages - Maintained responsive design across mobile and desktop - Added proper animate-pulse effects for realistic loading states ## Pages Updated - Public mentorship pages: /mentorship/programs/[key] and /mentorship/programs/[key]/modules/[key] - Internal mentorship pages: /my/mentorship/programs/[key] and related module pages - SearchPageLayout integration for consistent loading experience ## Accessibility Features - Proper ARIA roles (status, aria-label) for screen readers - Semantic HTML structure maintained - Focus management preserved - Responsive design intact ## Benefits - Improved perceived loading time by showing content structure - Better user experience with smooth transitions - Consistent loading states across mentorship portal - Enhanced accessibility compliance - Mobile-friendly skeleton layouts ## Technical Details - Components: ProgramCardSkeleton, ModuleCardSkeleton, MentorshipPageSkeleton - Updated: SkeletonBase.tsx, 4+ mentorship pages - Responsive: Mobile-first design with proper breakpoints - Accessibility: WCAG compliant with proper ARIA attributes Checklist: - I followed the contributing workflow - I verified that my code works as intended and resolves the issue as described - I ran make check-test locally: all warnings addressed, tests passed - I used AI for code, documentation, tests, or communication related to this PR
|
PR validation failed: No linked issue and no valid closing issue reference in PR description |
Contributor
|
Caution Review failedThe pull request is closed. Summary by CodeRabbit
WalkthroughThis pull request replaces the generic LoadingSpinner component with specialized skeleton UI components across mentorship pages. It introduces three new skeleton components—MentorshipPageSkeleton, ModuleCardSkeleton, and ProgramCardSkeleton—and updates SkeletonsBase and three page files to use MentorshipPageSkeleton during loading states. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 7 files
Confidence score: 4/5
- The only noted issue is a duplicate
case 'issues'infrontend/src/components/SkeletonsBase.tsx, which can make the mentorship issues skeleton unreachable and lead to the wrong skeleton being shown. - Given this is a single medium-severity UI logic issue, overall merge risk looks low-to-moderate and likely safe with a small fix.
- Pay close attention to
frontend/src/components/SkeletonsBase.tsx- duplicateissuescase causes the later branch to be unreachable.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/src/components/SkeletonsBase.tsx">
<violation number="1" location="frontend/src/components/SkeletonsBase.tsx:82">
P2: Duplicate `case 'issues'` makes the mentorship issues skeleton unreachable; `indexName === 'issues'` will always use the earlier CardSkeleton branch. Rename this case or update the existing `issues` case to avoid the collision.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return <MentorshipPageSkeleton type="programs" /> | ||
| case 'modules': | ||
| return <MentorshipPageSkeleton type="modules" /> | ||
| case 'issues': |
Contributor
There was a problem hiding this comment.
P2: Duplicate case 'issues' makes the mentorship issues skeleton unreachable; indexName === 'issues' will always use the earlier CardSkeleton branch. Rename this case or update the existing issues case to avoid the collision.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/SkeletonsBase.tsx, line 82:
<comment>Duplicate `case 'issues'` makes the mentorship issues skeleton unreachable; `indexName === 'issues'` will always use the earlier CardSkeleton branch. Rename this case or update the existing `issues` case to avoid the collision.</comment>
<file context>
@@ -74,6 +75,14 @@ const SkeletonBase = ({
+ return <MentorshipPageSkeleton type="programs" />
+ case 'modules':
+ return <MentorshipPageSkeleton type="modules" />
+ case 'issues':
+ return <MentorshipPageSkeleton type="issues" />
+ case 'mentees':
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Major UX improvement to enhance perceived loading time and user experience
Features Added
Technical Implementation
Pages Updated
Accessibility Features
Benefits
Technical Details
Checklist:
STOP AND READ BEFORE SUBMITTING! REMOVE THIS PARAGRAPH BEFORE OPENING THE PR
Thank you for your interest in contributing to OWASP Nest!
Before starting any work, all external contributors must first be assigned to an issue in the repository.
This is a mandatory step in the OWASP Nest workflow and ensures that effort is coordinated, approved, and tracked properly.
Exception: OWASP leaders are not required to follow this rule —- just make sure your username is included in the exception list.
If you were not assigned to the issue you are trying to resolve, stop right now and do NOT create this PR.
Unassigned pull requests are automatically closed by our workflows — please don't waste your time.
If you want to be assigned on any available issue, comment on it and wait for confirmation from the maintainers or project leads.
Proposed change
Resolves #(put the issue number here)
Add the PR description here.
Checklist
make check-testlocally: all warnings addressed, tests passed