Phase 3: Core Components with TDD Approach (MBA-45) in COG-GTM/angular2-hn#276
Open
devin-ai-integration[bot] wants to merge 8 commits into
Open
Phase 3: Core Components with TDD Approach (MBA-45) in COG-GTM/angular2-hn#276devin-ai-integration[bot] wants to merge 8 commits into
devin-ai-integration[bot] wants to merge 8 commits into
Conversation
- Replace Angular 9 with React 19, Vite 6, TypeScript 5.8 - Add React Router v7 for client-side routing - Add TanStack Query v5 for data fetching (replaces RxJS) - Create API service layer using official HN Firebase API - Port TypeScript interfaces from Angular models - Create SettingsContext (replaces SettingsService) - Port SCSS theme system (default, night, AMOLED black) - Implement all page components (Feed, ItemDetails, User) - Add shared components (Header, Footer, Settings, Loader, ErrorMessage) - Add ESLint 9 + Prettier for code quality - Add GitHub Actions CI (replaces Travis CI) - Update Firebase config for Vite output - Remove all Angular-specific files and dependencies Co-Authored-By: Matthew Guerra <matthew.guerra@cognition.ai>
Co-Authored-By: Matthew Guerra <matthew.guerra@cognition.ai>
Co-Authored-By: Matthew Guerra <matthew.guerra@cognition.ai>
- Set theme default to 'default' instead of '' to avoid flash of unstyled content on first visit (Devin Review) - Return hasMore flag from fetchFeed based on total IDs count instead of relying on items.length === 30, which breaks when deleted items are filtered out (Devin Review) Co-Authored-By: Matthew Guerra <matthew.guerra@cognition.ai>
The useEffect guard checked settings.theme which is always truthy
('default'), making the system color scheme detection dead code.
Check localStorage.getItem('theme') directly so first-time users
with no saved preference get the system dark/light mode applied.
Co-Authored-By: Matthew Guerra <matthew.guerra@cognition.ai>
…division by zero - Add APPLY_SYSTEM_THEME reducer action that updates state without writing to localStorage, so system dark mode detection stays active across page reloads until the user explicitly picks a theme - Guard poll bar width calculation against division by zero when poll_votes_count is 0 Co-Authored-By: Matthew Guerra <matthew.guerra@cognition.ai>
…ar 'second' - Check localStorage inside media query change handler so an explicit user theme choice stops system theme from overriding it mid-session - Fix '1 seconds ago' → '1 second ago' for consistency with other units Co-Authored-By: Matthew Guerra <matthew.guerra@cognition.ai>
- Add ErrorBoundary component with custom fallback support - Add Layout component wrapping Header, Footer, and ErrorBoundary - Refactor App.tsx to use Layout component - Add comprehensive test suites for Header, Footer, Loader, Settings, ErrorMessage, ErrorBoundary, Layout components - Add SettingsContext unit tests covering all state management - Add accessibility tests (vitest-axe) for all components - Configure Vitest with jsdom, CSS modules, and test setup - Add @testing-library/user-event, vitest-axe dev dependencies - Mock window.matchMedia for jsdom test environment - 48 tests across 9 test files, all passing MBA-45
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
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.
Summary
Implements Phase 3 of the Angular-to-React migration (Jira MBA-45): comprehensive TDD testing for all core UI components, plus new shared components (ErrorBoundary, Layout).
Built on top of the Phase 1/2 React infrastructure branch (
devin/1779402204-react-migration-infra), which already migrated the Angular app to React 19 + Vite + TypeScript with core components (Header, Footer, Settings, Loader, ErrorMessage) and the SettingsContext.What this PR adds:
New Components:
ErrorBoundary— Class component withgetDerivedStateFromErrorandcomponentDidCatch, supports custom fallback renderingLayout— Wraps Header + ErrorBoundary + main + Footer, applies theme class from SettingsContextRefactored:
App.tsx— Now usesLayoutcomponent instead of manually composing Header/FooterSettingsContext.tsxTest Infrastructure:
@testing-library/user-eventandvitest-axedev dependencies@testing-library/jest-dommatchers,vitest-axematchers, andwindow.matchMediamockTest Suites (48 tests across 9 files):
Header.test.tsx— Navigation links, logo, settings toggle, routing, accessibilityFooter.test.tsx— GitHub link attributes, content rendering, accessibilityLoader.test.tsx— Container/spinner rendering, accessibilitySettings.test.tsx— Theme radios, checkbox toggle, font/spacing inputs, localStorage persistence, accessibilityErrorMessage.test.tsx— Message rendering, rerendering, accessibilityErrorBoundary.test.tsx— Error catching, default/custom fallback, console loggingLayout.test.tsx— Composition, main element, theme class, error boundary integration, accessibilitySettingsContext.test.tsx— All state actions, localStorage persistence/loading, provider requirementReview & Testing Checklist for Human
npm test— all 48 tests should passnpm run typecheck— should pass with 0 errorsnpm run lint— should pass (1 pre-existing warning in SettingsContext.tsx about fast refresh)npm run devand verify the app loads correctly at localhost:4200Notes
HTMLCanvasElement.prototype.getContextwarnings in test stderr are a known jsdom limitation with axe-core's color contrast checks — they don't affect test resultsreact-refresh/only-export-componentsin SettingsContext.tsx) is pre-existing from the base branch and not introduced by this PRLink to Devin session: https://app.devin.ai/sessions/fe76dcad05f3489ebd65c81fa8967d06
Requested by: @bcmake
Devin Review