Revert recent 5 PR merges - #810
Merged
Merged
Conversation
|
Hi @Aditya948351, I guess you need to update your PR so that all CI checks get passed! |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR removes several roadmap/progress-related utilities and UI components, simplifies learning progress to only work for authenticated users via Firestore, and replaces centralized auth prompt constants with inline strings in multiple UI flows.
Changes:
- Removed the pace predictor utility/component (and associated tests) plus the roadmap checklist UI.
- Updated
useLearningProgressto drop guest (localStorage) progress persistence and reset functionality; progress interactions become auth-only. - Simplified community showcase fetching (removed cursor pagination) and updated various auth-required actions to use inline alert strings; adjusted UI/tests accordingly.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/pacePredictor.ts | Removed pace predictor calculation logic and roadmap pace data. |
| src/utils/tests/pacePredictor.test.ts | Removed unit tests for pace predictor behavior. |
| src/lib/constants.ts | Removed centralized AUTH_MESSAGES string constants. |
| src/hooks/useLearningProgress.ts | Dropped guest persistence/reset; toggling now no-ops for unauthenticated users and relies solely on Firestore writes. |
| src/components/resources/InternshipCalendarModal.tsx | Replaced AUTH_MESSAGES usage with inline auth alert string. |
| src/components/projects/ProjectCardSkeleton.tsx | Removed skeleton component previously used for loading states. |
| src/components/projects/ProjectCard.tsx | Replaced AUTH_MESSAGES usage with inline auth alert string. |
| src/components/profile/UserProfile.tsx | Replaced centralized auth message; inlined project loading skeleton markup. |
| src/components/profile/FollowButton.tsx | Replaced AUTH_MESSAGES usage with inline auth alert string. |
| src/components/layout/tests/SearchModal.test.tsx | Simplified tests by removing fake timers and multiple act wrappers. |
| src/components/features/tests/LearningPacePredictor.test.tsx | Removed component tests for the pace predictor feature. |
| src/components/features/SkillTreeVisualizer.tsx | Added overlay-close listener and keyboard navigation; changed guest copy; hid completion toggle for guests. |
| src/components/features/RoadmapChecklist.tsx | Removed checklist tracker component. |
| src/components/features/LearningPacePredictor.tsx | Removed pace predictor UI component. |
| src/components/common/Pagination.tsx | Removed pagination component used by community showcase. |
| src/app/u/client.tsx | Replaced AUTH_MESSAGES usage with inline auth alert string. |
| src/app/roadmaps/[id]/page.tsx | Removed checklist section wiring and node data from roadmap page. |
| src/app/progress/page.tsx | Removed pace predictor from progress page and simplified layout/typography. |
| src/app/opensource/page.tsx | Replaced AUTH_MESSAGES usage with inline auth alert string. |
| src/app/community/view/client.tsx | Replaced AUTH_MESSAGES usage with inline auth alert string(s). |
| src/app/community/page.tsx | Removed cursor pagination logic; switched to fixed-limit project query; replaced AUTH_MESSAGES usage with inline strings and simplified loading UI. |
Comments suppressed due to low confidence (1)
src/components/features/SkillTreeVisualizer.tsx:180
- The
close-all-overlaysevent listener and theuseKeyboardShortcutsbindings are duplicated (the same blocks already exist later in the component). This will register two window listeners and fire arrow key handlers twice (e.g., skipping nodes). Remove one set of these effect/shortcut blocks so each is registered exactly once.
// Listen for the escape close-all-overlays event to close the side drawer
useEffect(() => {
const handleCloseAll = () => {
setSelectedNode(null);
};
window.addEventListener('close-all-overlays', handleCloseAll);
return () =>
window.removeEventListener('close-all-overlays', handleCloseAll);
}, []);
// Bind local arrow key shortcuts for node selection cycling
useKeyboardShortcuts({
arrowright: () => {
if (nodes.length === 0) return;
const currentIndex = selectedNode
? nodes.findIndex((n) => n.id === selectedNode.id)
: -1;
const nextIndex =
currentIndex === -1 ? 0 : (currentIndex + 1) % nodes.length;
setSelectedNode(nodes[nextIndex]);
},
arrowleft: () => {
if (nodes.length === 0) return;
const currentIndex = selectedNode
? nodes.findIndex((n) => n.id === selectedNode.id)
: -1;
const prevIndex =
currentIndex === -1
? nodes.length - 1
: (currentIndex - 1 + nodes.length) % nodes.length;
setSelectedNode(nodes[prevIndex]);
},
});
// Listen for the escape close-all-overlays event to close the side drawer
useEffect(() => {
const handleCloseAll = () => {
setSelectedNode(null);
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+351
to
+359
| {user && ( | ||
| <button | ||
| aria-label="Toggle node complete status" | ||
| className="mb-1 hover:scale-110 active:scale-95 transition-transform" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| toggleNode(activePath, node.id); | ||
| }} | ||
| > |
Comment on lines
+46
to
47
| const fetchData = async () => { | ||
| setLoading(true); |
Comment on lines
96
to
98
| useEffect(() => { | ||
| // Any time the tab or sort option changes, pagination resets to page 1. | ||
| setPageCursors([]); | ||
| setLastDocInPage(null); | ||
| setCurrentPage(1); | ||
| fetchData(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [activeTab, sortOption]); |
Comment on lines
81
to
89
| it('filters results based on query', () => { | ||
| render(<SearchModal />); | ||
| act(() => { | ||
| fireEvent.keyDown(window, { ctrlKey: true, key: 'k' }); | ||
| }); | ||
| fireEvent.keyDown(window, { ctrlKey: true, key: 'k' }); | ||
|
|
||
| const input = screen.getByPlaceholderText(/Search wiki articles/); | ||
| act(() => { | ||
| fireEvent.change(input, { target: { value: 'react' } }); | ||
| jest.advanceTimersByTime(300); // Advance debounce timer | ||
| }); | ||
| fireEvent.change(input, { target: { value: 'react' } }); | ||
|
|
||
| expect(screen.getByText('Full Stack React Guide')).toBeInTheDocument(); | ||
| }); |
Comment on lines
+47
to
+51
| const actualCompletedNodes = user ? completedNodes : []; | ||
| const actualLoading = user ? loading : false; | ||
|
|
||
| // ── Toggle a node (complete / incomplete) ───────────────────────────────── | ||
| const toggleNode = useCallback( | ||
| async (pathId: string, nodeId: string) => { | ||
| const nodeKey = `${pathId}-${nodeId}`; | ||
| const isCompleted = actualCompletedNodes.includes(nodeKey); | ||
| const nextCompletedNodes = isCompleted | ||
| ? actualCompletedNodes.filter((id) => id !== nodeKey) | ||
| : [...actualCompletedNodes, nodeKey]; | ||
|
|
||
| if (!user) { | ||
| // Guest: persist to localStorage and update local state immediately | ||
| const localData = readLocalProgress(); | ||
| if (isCompleted) { | ||
| delete localData[nodeKey]; | ||
| } else { | ||
| localData[nodeKey] = true; | ||
| } | ||
| writeLocalProgress(localData); | ||
| setCompletedNodes(nextCompletedNodes); | ||
| return; | ||
| } | ||
|
|
||
| // Authenticated: persist to Firestore (latency compensation via onSnapshot) | ||
| try { | ||
| const docRef = doc(db, 'user_progress', user.uid); | ||
| await setDoc( | ||
| docRef, | ||
| { | ||
| userId: user.uid, | ||
| completedNodes: nextCompletedNodes, | ||
| updatedAt: new Date().toISOString(), | ||
| }, | ||
| { merge: true } | ||
| ); | ||
| } catch (error) { | ||
| console.error('Failed to save learning progress:', error); | ||
| } | ||
| }, | ||
| [user, actualCompletedNodes] | ||
| ); | ||
|
|
||
| // ── Reset progress for a specific path (or all) ─────────────────────────── | ||
| const resetProgress = useCallback( | ||
| async (pathId?: string) => { | ||
| if (!user) { | ||
| // Guest: clear from localStorage | ||
| if (pathId) { | ||
| const localData = readLocalProgress(); | ||
| const filtered = Object.fromEntries( | ||
| Object.entries(localData).filter(([key]) => !key.startsWith(`${pathId}-`)) | ||
| ); | ||
| writeLocalProgress(filtered); | ||
| setCompletedNodes((prev) => prev.filter((key) => !key.startsWith(`${pathId}-`))); | ||
| } else { | ||
| writeLocalProgress({}); | ||
| setCompletedNodes([]); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // Authenticated: reset in Firestore | ||
| try { | ||
| const docRef = doc(db, 'user_progress', user.uid); | ||
| const nextNodes = pathId | ||
| ? actualCompletedNodes.filter((key) => !key.startsWith(`${pathId}-`)) | ||
| : []; | ||
| await setDoc( | ||
| docRef, | ||
| { | ||
| userId: user.uid, | ||
| completedNodes: nextNodes, | ||
| updatedAt: new Date().toISOString(), | ||
| }, | ||
| { merge: true } | ||
| ); | ||
| } catch (error) { | ||
| console.error('Failed to reset learning progress:', error); | ||
| } | ||
| }, | ||
| [user, actualCompletedNodes] | ||
| ); | ||
| const toggleNode = async (pathId: string, nodeId: string) => { | ||
| if (!user) return; |
Comment on lines
+79
to
+81
| const isNodeCompleted = (pathId: string, nodeId: string) => { | ||
| return actualCompletedNodes.includes(`${pathId}-${nodeId}`); | ||
| }; |
| e.stopPropagation(); | ||
| if (!user) { | ||
| alert(AUTH_MESSAGES.LOGIN_TO_STAR_PROJECTS); | ||
| alert('Please login to star projects.'); |
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
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.
Reverting 5 PRs as requested.