From 27eb0280a88caf77661c21f4bfbc28c8a314c48d Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 21:17:55 -0500 Subject: [PATCH 01/15] docs(specs): add scroll-to-top button specification - Create feature spec for scroll-to-top button fixed to bottom-right - Define 3 user stories with prioritized user journeys - Specify 7 functional requirements for visibility and behavior - Add 5 measurable success criteria for performance and usability - Document edge cases for responsive behavior and accessibility - Include quality validation checklist Co-authored-by: Qwen-Coder --- .../checklists/requirements.md | 35 +++++++ specs/001-scroll-to-top/spec.md | 91 +++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 specs/001-scroll-to-top/checklists/requirements.md create mode 100644 specs/001-scroll-to-top/spec.md diff --git a/specs/001-scroll-to-top/checklists/requirements.md b/specs/001-scroll-to-top/checklists/requirements.md new file mode 100644 index 0000000..7ec2e5e --- /dev/null +++ b/specs/001-scroll-to-top/checklists/requirements.md @@ -0,0 +1,35 @@ +# Specification Quality Checklist: Scroll to Top Button + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-03-06 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items passed validation on first review +- Specification is ready for planning phase diff --git a/specs/001-scroll-to-top/spec.md b/specs/001-scroll-to-top/spec.md new file mode 100644 index 0000000..e9d8694 --- /dev/null +++ b/specs/001-scroll-to-top/spec.md @@ -0,0 +1,91 @@ +# Feature Specification: Scroll to Top Button + +**Feature Branch**: `001-scroll-to-top` +**Created**: 2026-03-06 +**Status**: Draft +**Input**: User description: "scroll to top button fixed to bottom-right above XL breakpoint" + +## User Scenarios & Testing + +### User Story 1 - Scroll to Top from Bottom of Page (Priority: P1) + +As a user viewing long diff content, I want to quickly return to the top of the page without manually scrolling, so I can save time and effort when comparing lengthy text differences. + +**Why this priority**: This is the core functionality that delivers immediate value. Users working with long diff comparisons need efficient navigation, and manual scrolling through lengthy content is tedious and time-consuming. + +**Independent Test**: User can scroll down a page with long content, see a scroll-to-top button appear, click it, and the page smoothly scrolls to the top. + +**Acceptance Scenarios**: + +1. **Given** the page has been scrolled down significantly, **When** the user clicks the scroll-to-top button, **Then** the page smoothly scrolls to the top +2. **Given** the user is viewing a long diff comparison, **When** they scroll down past a threshold, **Then** the scroll-to-top button becomes visible +3. **Given** the page is at the top position, **When** the user views the page, **Then** the scroll-to-top button is hidden + +--- + +### User Story 2 - Responsive Visibility Based on Screen Size (Priority: P2) + +As a user on a large desktop screen, I want the scroll-to-top button to appear only when viewing on sufficiently large screens, so the feature is available when screen real estate allows without cluttering smaller mobile views. + +**Why this priority**: This ensures the feature enhances the desktop experience without impacting mobile usability. On smaller screens, the button could obstruct content, while on larger screens it provides convenient navigation. + +**Independent Test**: On screens above the XL breakpoint, the button appears when scrolled down. On screens below XL, the button never appears regardless of scroll position. + +**Acceptance Scenarios**: + +1. **Given** the viewport width is above the XL breakpoint, **When** the user scrolls down, **Then** the scroll-to-top button appears +2. **Given** the viewport width is below the XL breakpoint, **When** the user scrolls down, **Then** the scroll-to-top button remains hidden +3. **Given** the user resizes the browser from below XL to above XL, **When** they scroll down, **Then** the button appears as expected + +--- + +### User Story 3 - Fixed Positioning for Easy Access (Priority: P3) + +As a user navigating long content, I want the scroll-to-top button to remain in a consistent fixed position at the bottom-right of the screen, so I can easily locate and click it without searching. + +**Why this priority**: Consistent positioning improves usability and reduces cognitive load. Users know exactly where to find the button regardless of how far they've scrolled. + +**Independent Test**: The button remains fixed at the bottom-right corner of the viewport as the user scrolls, maintaining its position relative to the screen rather than the page content. + +**Acceptance Scenarios**: + +1. **Given** the scroll-to-top button is visible, **When** the user scrolls up or down, **Then** the button stays fixed in the bottom-right corner of the viewport +2. **Given** the page content is shorter than the viewport height, **When** the user views the page, **Then** the scroll-to-top button does not appear (no scrolling needed) + +--- + +### Edge Cases + +- What happens when the page content is shorter than the viewport height? (Button should not appear since no scrolling is needed) +- How does the system handle rapid scrolling up and down? (Button should appear/disappear smoothly without flickering) +- What happens when the user resizes the browser window crossing the XL breakpoint threshold? (Button visibility should update appropriately) +- How does the button behave with browser zoom enabled? (Should remain visible and functional at various zoom levels) +- What happens if the user has reduced motion preferences enabled? (Scroll animation should respect user preferences) + +## Requirements + +### Functional Requirements + +- **FR-001**: System MUST display a scroll-to-top button when the page is scrolled beyond a minimum threshold distance from the top +- **FR-002**: System MUST hide the scroll-to-top button when the page is at or near the top position +- **FR-003**: System MUST display the scroll-to-top button only when the viewport width is at or above the XL breakpoint +- **FR-004**: System MUST position the scroll-to-top button in a fixed location at the bottom-right corner of the viewport +- **FR-005**: System MUST scroll the page to the top when the user clicks the scroll-to-top button +- **FR-006**: System MUST provide visual feedback indicating the button is clickable (hover state) +- **FR-007**: System MUST respect user preferences for reduced motion when animating the scroll behavior + +### Key Entities + +- **Scroll Position**: The current vertical offset of the page content relative to the viewport top +- **Viewport Breakpoint**: The minimum screen width threshold (XL) at which the button becomes available +- **Scroll Threshold**: The minimum scroll distance at which the button appears + +## Success Criteria + +### Measurable Outcomes + +- **SC-001**: Users can return to the top of the page with a single click from any scroll position +- **SC-002**: The scroll-to-top button appears within 100 milliseconds of scrolling past the threshold +- **SC-003**: Page scroll animation to top completes within 500 milliseconds +- **SC-004**: 100% of users can successfully locate and activate the scroll-to-top feature on screens above XL breakpoint +- **SC-005**: Button visibility correctly responds to viewport resize events within 50 milliseconds From d02913a5fa7a23cb4487facf835c1e065c138eee Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 21:27:09 -0500 Subject: [PATCH 02/15] docs(specs): add clarifications for scroll-to-top button - Document button content: upward arrow icon (Unicode/HTML entity) - Define scroll threshold: 50vh (half viewport height) for responsive behavior - Update FR-001 and add FR-008 with clarified requirements - Update Key Entities section with threshold specification - Add edge case for dynamic threshold recalculation on resize Co-authored-by: Qwen-Coder --- specs/001-scroll-to-top/spec.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/specs/001-scroll-to-top/spec.md b/specs/001-scroll-to-top/spec.md index e9d8694..d180630 100644 --- a/specs/001-scroll-to-top/spec.md +++ b/specs/001-scroll-to-top/spec.md @@ -5,6 +5,13 @@ **Status**: Draft **Input**: User description: "scroll to top button fixed to bottom-right above XL breakpoint" +## Clarifications + +### Session 2026-03-06 + +- Q: What icon or content should the scroll-to-top button display? → A: Upward arrow icon using Unicode or HTML entity +- Q: What scroll threshold should trigger button visibility? → A: 50vh (half viewport height) + ## User Scenarios & Testing ### User Story 1 - Scroll to Top from Bottom of Page (Priority: P1) @@ -61,24 +68,26 @@ As a user navigating long content, I want the scroll-to-top button to remain in - What happens when the user resizes the browser window crossing the XL breakpoint threshold? (Button visibility should update appropriately) - How does the button behave with browser zoom enabled? (Should remain visible and functional at various zoom levels) - What happens if the user has reduced motion preferences enabled? (Scroll animation should respect user preferences) +- How does the 50vh threshold adapt when viewport height changes? (Threshold should recalculate dynamically on resize) ## Requirements ### Functional Requirements -- **FR-001**: System MUST display a scroll-to-top button when the page is scrolled beyond a minimum threshold distance from the top +- **FR-001**: System MUST display a scroll-to-top button when the page is scrolled beyond a minimum threshold distance from the top (50vh - half viewport height) - **FR-002**: System MUST hide the scroll-to-top button when the page is at or near the top position - **FR-003**: System MUST display the scroll-to-top button only when the viewport width is at or above the XL breakpoint - **FR-004**: System MUST position the scroll-to-top button in a fixed location at the bottom-right corner of the viewport - **FR-005**: System MUST scroll the page to the top when the user clicks the scroll-to-top button - **FR-006**: System MUST provide visual feedback indicating the button is clickable (hover state) - **FR-007**: System MUST respect user preferences for reduced motion when animating the scroll behavior +- **FR-008**: System MUST display an upward arrow icon (Unicode or HTML entity) as the button content ### Key Entities - **Scroll Position**: The current vertical offset of the page content relative to the viewport top - **Viewport Breakpoint**: The minimum screen width threshold (XL) at which the button becomes available -- **Scroll Threshold**: The minimum scroll distance at which the button appears +- **Scroll Threshold**: The minimum scroll distance at which the button appears (50vh - half viewport height) ## Success Criteria From 5e380094aa0d832c2bfe2f83b447099a8196b5d5 Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 21:34:25 -0500 Subject: [PATCH 03/15] docs(specs): add clarifications for button shape, accessibility, and spacing - Define button shape: circular (40-48px diameter) - Define accessibility: WCAG 2.1 AA (keyboard, focus, ARIA, screen reader) - Define button offset: 16px from bottom and right edges - Update FR-004 with spacing specification - Add FR-009 for circular shape requirement - Add FR-010 for WCAG 2.1 AA accessibility requirement Co-authored-by: Qwen-Coder --- specs/001-scroll-to-top/spec.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/specs/001-scroll-to-top/spec.md b/specs/001-scroll-to-top/spec.md index d180630..332cf69 100644 --- a/specs/001-scroll-to-top/spec.md +++ b/specs/001-scroll-to-top/spec.md @@ -11,6 +11,9 @@ - Q: What icon or content should the scroll-to-top button display? → A: Upward arrow icon using Unicode or HTML entity - Q: What scroll threshold should trigger button visibility? → A: 50vh (half viewport height) +- Q: What should the button's physical shape and size be? → A: Circular (40-48px diameter) +- Q: What accessibility features should the button support? → A: Full WCAG 2.1 AA (keyboard, focus, ARIA, screen reader) +- Q: What spacing/offset should the button have from the bottom and right edges? → A: 16px offset (Tailwind p-4) ## User Scenarios & Testing @@ -77,11 +80,13 @@ As a user navigating long content, I want the scroll-to-top button to remain in - **FR-001**: System MUST display a scroll-to-top button when the page is scrolled beyond a minimum threshold distance from the top (50vh - half viewport height) - **FR-002**: System MUST hide the scroll-to-top button when the page is at or near the top position - **FR-003**: System MUST display the scroll-to-top button only when the viewport width is at or above the XL breakpoint -- **FR-004**: System MUST position the scroll-to-top button in a fixed location at the bottom-right corner of the viewport +- **FR-004**: System MUST position the scroll-to-top button in a fixed location at the bottom-right corner of the viewport with 16px offset from both bottom and right edges - **FR-005**: System MUST scroll the page to the top when the user clicks the scroll-to-top button - **FR-006**: System MUST provide visual feedback indicating the button is clickable (hover state) - **FR-007**: System MUST respect user preferences for reduced motion when animating the scroll behavior - **FR-008**: System MUST display an upward arrow icon (Unicode or HTML entity) as the button content +- **FR-009**: System MUST render the button as a circular shape (40-48px diameter) with styling consistent with the existing design system +- **FR-010**: System MUST support WCAG 2.1 AA accessibility: keyboard navigation (Enter/Space), visible focus ring, ARIA label "Scroll to top", and screen reader compatibility ### Key Entities From 21131c9d1c5fd92a257fbc88e7a8a601e4c83bc5 Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 21:38:03 -0500 Subject: [PATCH 04/15] docs(specs): clarify cursor pointer and hover effects for button - Add clarification: cursor pointer and hover background color change - Update FR-006 with explicit hover behavior specification Co-authored-by: Qwen-Coder --- specs/001-scroll-to-top/spec.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/001-scroll-to-top/spec.md b/specs/001-scroll-to-top/spec.md index 332cf69..6dbaff9 100644 --- a/specs/001-scroll-to-top/spec.md +++ b/specs/001-scroll-to-top/spec.md @@ -14,6 +14,7 @@ - Q: What should the button's physical shape and size be? → A: Circular (40-48px diameter) - Q: What accessibility features should the button support? → A: Full WCAG 2.1 AA (keyboard, focus, ARIA, screen reader) - Q: What spacing/offset should the button have from the bottom and right edges? → A: 16px offset (Tailwind p-4) +- Q: Should the button have cursor pointer and hover effects? → A: Yes - cursor pointer and hover background color change ## User Scenarios & Testing @@ -82,7 +83,7 @@ As a user navigating long content, I want the scroll-to-top button to remain in - **FR-003**: System MUST display the scroll-to-top button only when the viewport width is at or above the XL breakpoint - **FR-004**: System MUST position the scroll-to-top button in a fixed location at the bottom-right corner of the viewport with 16px offset from both bottom and right edges - **FR-005**: System MUST scroll the page to the top when the user clicks the scroll-to-top button -- **FR-006**: System MUST provide visual feedback indicating the button is clickable (hover state) +- **FR-006**: System MUST provide visual feedback indicating the button is clickable: cursor pointer on hover and background color change on hover - **FR-007**: System MUST respect user preferences for reduced motion when animating the scroll behavior - **FR-008**: System MUST display an upward arrow icon (Unicode or HTML entity) as the button content - **FR-009**: System MUST render the button as a circular shape (40-48px diameter) with styling consistent with the existing design system From a4f6602c0b8aea1b53e35547d0afcde476080210 Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 21:46:36 -0500 Subject: [PATCH 05/15] docs(plan): add implementation plan for scroll-to-top button - Create implementation plan with technical context and constitution check - Add research document: scroll handling, smooth scroll API, WCAG 2.1 AA, Tailwind - Add quickstart guide with implementation steps - Update QWEN.md agent context with feature technologies - All constitution gates pass (client-side, test coverage, accessibility, types, simplicity) Co-authored-by: Qwen-Coder --- QWEN.md | 5 + specs/001-scroll-to-top/plan.md | 233 ++++++++++++++++++++++++++ specs/001-scroll-to-top/quickstart.md | 123 ++++++++++++++ specs/001-scroll-to-top/research.md | 179 ++++++++++++++++++++ 4 files changed, 540 insertions(+) create mode 100644 specs/001-scroll-to-top/plan.md create mode 100644 specs/001-scroll-to-top/quickstart.md create mode 100644 specs/001-scroll-to-top/research.md diff --git a/QWEN.md b/QWEN.md index 6a03c9d..910a6a7 100644 --- a/QWEN.md +++ b/QWEN.md @@ -4,6 +4,9 @@ Auto-generated from all feature plans. Last updated: 2026-03-03 ## Active Technologies +- TypeScript 5 (strict mode) + React 19, Tailwind CSS 4 (001-scroll-to-top) +- N/A (no persistence) (001-scroll-to-top) + - TypeScript 5 (strict mode) + React 19, diff library (v8) (001-fix-diff-gutter) ## Project Structure @@ -23,6 +26,8 @@ TypeScript 5 (strict mode): Follow standard conventions ## Recent Changes +- 001-scroll-to-top: Added TypeScript 5 (strict mode) + React 19, Tailwind CSS 4 + - 001-fix-diff-gutter: Restructured DiffViewer to use CSS grid rows with inline line numbers, ensuring line numbers always match content height when text wraps. Removed separate LineNumberGutter component from unified view and unused scroll sync logic. diff --git a/specs/001-scroll-to-top/plan.md b/specs/001-scroll-to-top/plan.md new file mode 100644 index 0000000..8dc3089 --- /dev/null +++ b/specs/001-scroll-to-top/plan.md @@ -0,0 +1,233 @@ +# Implementation Plan: Scroll to Top Button + +**Branch**: `001-scroll-to-top` | **Date**: 2026-03-06 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from `/specs/001-scroll-to-top/spec.md` + +**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/templates/plan-template.md` for the execution workflow. + +## Summary + +Implement a scroll-to-top button that appears when users scroll down past 50vh on screens ≥1280px (XL breakpoint). The button is fixed at bottom-right (16px offset), circular (40-48px), displays an upward arrow icon (Unicode/HTML entity), and provides smooth scroll-to-top functionality with full WCAG 2.1 AA accessibility support. + +## Technical Context + +**Language/Version**: TypeScript 5 (strict mode) +**Primary Dependencies**: React 19, Tailwind CSS 4 +**Storage**: N/A (no persistence) +**Testing**: Vitest 4 with @testing-library/react and @testing-library/user-event +**Target Platform**: Modern browsers (desktop ≥1280px width) +**Project Type**: Desktop web application (client-side only SPA) +**Performance Goals**: Button appears within 100ms, scroll animation completes within 500ms +**Constraints**: Client-side only, 100% test coverage required, WCAG 2.1 AA accessible +**Scale/Scope**: Single component feature, ~100-200 lines of code + +## Constitution Check + +_GATE: Must pass before Phase 0 research. Re-check after Phase 1 design._ + +| Principle | Status | Notes | +| ----------------------------- | ------- | ------------------------------------------------------- | +| I. Client-Side Only | ✅ PASS | All logic executes in browser, no backend | +| II. Full Test Coverage | ✅ PASS | Component + hook tests with 100% coverage required | +| III. Accessibility First | ✅ PASS | WCAG 2.1 AA explicitly required (keyboard, ARIA, focus) | +| IV. Type Safety | ✅ PASS | TypeScript strict mode, explicit interfaces | +| V. Simplicity and Performance | ✅ PASS | Single component, no state libraries, Tailwind only | + +**Verdict**: All gates pass. Proceeding to Phase 0. + +## Project Structure + +### Documentation (this feature) + +```text +specs/001-scroll-to-top/ +├── plan.md # This file +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output (N/A - no data model) +├── quickstart.md # Phase 1 output +├── contracts/ # Phase 1 output (N/A - no external interfaces) +└── tasks.md # Phase 2 output (/speckit.tasks command) +``` + +### Source Code (repository root) + +```text +src/ +├── components/ +│ └── ScrollToTop/ +│ ├── ScrollToTop.tsx +│ ├── ScrollToTop.types.ts +│ ├── ScrollToTop.test.tsx +│ └── index.ts +├── hooks/ +│ ├── useScrollPosition.ts +│ └── useScrollPosition.test.ts +└── ...existing structure + +tests/ +└── ...existing structure (tests co-located with source) +``` + +**Structure Decision**: Following established project pattern for components (matching ViewToggle, TextInput, DiffMethodToggle structure) and hooks (matching useMediaQuery, useLocalStorage pattern). + +## Complexity Tracking + +No constitution violations. Complexity tracking N/A. + +## Phase 0: Research & Discovery + +### Research Topics + +1. **Scroll event handling best practices in React** + - Passive event listeners for performance + - Debouncing/throttling strategies + - requestAnimationFrame usage + +2. **Smooth scroll behavior** + - Native `scrollTo({ behavior: 'smooth' })` support + - Fallback strategies for reduced motion preferences + - Cross-browser compatibility + +3. **WCAG 2.1 AA button requirements** + - Keyboard interaction patterns (Enter/Space) + - Focus indicator requirements + - ARIA labeling best practices + +4. **Tailwind CSS 4 fixed positioning** + - Utility classes for fixed positioning + - Responsive breakpoint utilities (xl = 1280px) + - Dark mode support patterns + +### Research Dispatch + +```text +Task 1: Research scroll event handling patterns in React 19 +Task 2: Research smooth scroll API and reduced motion media query +Task 3: Research WCAG 2.1 AA button compliance requirements +Task 4: Research Tailwind CSS 4 positioning utilities +``` + +## Phase 1: Design & Contracts + +### Data Model + +N/A - This feature has no data entities. It's a UI component with behavioral logic. + +### Interface Contracts + +N/A - This feature exposes no external interfaces. It's an internal UI component. + +### Component API Design + +**ScrollToTop Component** + +```typescript +interface ScrollToTopProps { + // No props - component is self-contained +} +``` + +**useScrollPosition Hook** + +```typescript +interface UseScrollPositionOptions { + threshold: number | '50vh'; // Scroll threshold for visibility +} + +function useScrollPosition(options?: UseScrollPositionOptions): { + isScrolledPastThreshold: boolean; + scrollY: number; +}; +``` + +### Quickstart + +```bash +# 1. Create component files +mkdir -p src/components/ScrollToTop + +# 2. Create hook files +touch src/hooks/useScrollPosition.ts + +# 3. Run development server +npm start + +# 4. Run tests +npm test + +# 5. Verify coverage +npm run test:ci +``` + +## Phase 2: Implementation Tasks + +### Task Breakdown + +1. **Create useScrollPosition hook** + - Track scroll Y position + - Calculate 50vh threshold dynamically + - Return isScrolledPastThreshold boolean + +2. **Create ScrollToTop component** + - Fixed positioning (bottom-right, 16px offset) + - Circular shape (40-48px) + - Upward arrow icon (Unicode ▲ or similar) + - Hover states (cursor pointer, background change) + - Focus ring for accessibility + - ARIA label "Scroll to top" + - Keyboard handler (Enter/Space) + +3. **Integrate into App component** + - Import ScrollToTop + - Render in App layout + +4. **Write tests** + - Hook tests (scroll threshold logic) + - Component tests (rendering, interactions, accessibility) + +5. **Verify quality gates** + - Lint: `npm run lint` + - Type check: `npm run lint:tsc` + - Tests: `npm run test:ci` (100% coverage) + - Build: `npm run build` + +## Phase 3: Validation + +### Acceptance Criteria + +- [ ] Button appears when scrolled past 50vh +- [ ] Button hidden when at top of page +- [ ] Button only visible on screens ≥1280px (XL) +- [ ] Button fixed at bottom-right with 16px offset +- [ ] Button is circular (40-48px diameter) +- [ ] Upward arrow icon displayed +- [ ] Cursor pointer on hover +- [ ] Background color change on hover +- [ ] Keyboard accessible (Tab, Enter/Space) +- [ ] Visible focus ring +- [ ] ARIA label "Scroll to top" +- [ ] Respects reduced motion preferences +- [ ] 100% test coverage maintained +- [ ] All quality gates pass + +### Risk Assessment + +| Risk | Likelihood | Impact | Mitigation | +| ---------------------------------- | ---------- | ------ | ---------------------------------------------------- | +| Scroll event performance | Low | Medium | Use passive listener, throttle/debounce | +| Cross-browser smooth scroll | Low | Low | Native API has wide support, reduced motion fallback | +| Focus ring visibility in dark mode | Low | Low | Test in both light/dark modes | + +## Constitution Check (Post-Design) + +_Re-evaluate after design complete._ + +| Principle | Status | Notes | +| ----------------------------- | ------- | ----------------------------- | +| I. Client-Side Only | ✅ PASS | Confirmed | +| II. Full Test Coverage | ✅ PASS | Tests planned for all code | +| III. Accessibility First | ✅ PASS | WCAG 2.1 AA built into design | +| IV. Type Safety | ✅ PASS | Interfaces defined | +| V. Simplicity and Performance | ✅ PASS | Minimal implementation | + +**Verdict**: All gates pass. Proceeding to implementation. diff --git a/specs/001-scroll-to-top/quickstart.md b/specs/001-scroll-to-top/quickstart.md new file mode 100644 index 0000000..07d5bd0 --- /dev/null +++ b/specs/001-scroll-to-top/quickstart.md @@ -0,0 +1,123 @@ +# Quickstart: Scroll to Top Button + +**Feature**: Scroll to Top Button +**Branch**: 001-scroll-to-top + +## Overview + +This feature adds a scroll-to-top button that appears when users scroll down past 50vh on screens ≥1280px (XL breakpoint). + +## File Structure + +``` +src/ +├── components/ +│ └── ScrollToTop/ +│ ├── ScrollToTop.tsx # Main component +│ ├── ScrollToTop.types.ts # TypeScript types +│ ├── ScrollToTop.test.tsx # Component tests +│ └── index.ts # Public exports +└── hooks/ + ├── useScrollPosition.ts # Scroll position hook + └── useScrollPosition.test.ts # Hook tests +``` + +## Implementation Steps + +### 1. Create the Hook + +Create `src/hooks/useScrollPosition.ts`: + +- Use `useSyncExternalStore` pattern (like `useMediaQuery`) +- Track scroll Y position with passive event listener +- Calculate 50vh threshold dynamically +- Return `isScrolledPastThreshold` boolean + +### 2. Create the Component + +Create `src/components/ScrollToTop/ScrollToTop.tsx`: + +- Fixed positioning: `fixed bottom-4 right-4` +- Responsive visibility: `hidden xl:block` +- Circular shape: `rounded-full h-12 w-12` +- Upward arrow icon: Unicode `▲` or HTML entity `Δ` +- Hover states: `cursor-pointer` + background color change +- Focus ring: `focus:ring-2 focus:ring-blue-500` +- ARIA label: `aria-label="Scroll to top"` +- Keyboard handler: Enter/Space triggers scroll + +### 3. Create Types + +Create `src/components/ScrollToTop/ScrollToTop.types.ts`: + +- Define `ScrollToTopProps` interface (empty for this component) + +### 4. Create Index + +Create `src/components/ScrollToTop/index.ts`: + +- Export component as default +- Export types if needed + +### 5. Write Tests + +Create `src/components/ScrollToTop/ScrollToTop.test.tsx`: + +- Test rendering (hidden at top, visible when scrolled) +- Test responsive visibility (hidden below XL) +- Test click behavior (scrolls to top) +- Test keyboard accessibility (Enter/Space) +- Test ARIA attributes + +Create `src/hooks/useScrollPosition.test.ts`: + +- Test threshold calculation +- Test scroll position tracking + +### 6. Integrate into App + +Update `src/components/App/App.tsx`: + +- Import `ScrollToTop` component +- Render `` in the component tree + +### 7. Verify Quality Gates + +```bash +# Lint +npm run lint + +# Type check +npm run lint:tsc + +# Tests with coverage +npm run test:ci + +# Production build +npm run build +``` + +All must pass with 100% test coverage. + +## Key Technical Details + +- **Scroll threshold**: 50vh (half viewport height, dynamic) +- **Breakpoint**: XL (1280px) - Tailwind default +- **Button size**: 48px diameter (h-12 w-12) +- **Offset**: 16px from bottom and right (bottom-4 right-4) +- **Icon**: Unicode upward arrow (▲) +- **Animation**: Native `scrollTo({ behavior: 'smooth' })` +- **Accessibility**: WCAG 2.1 AA compliant + +## Testing Checklist + +- [ ] Button hidden at page top +- [ ] Button visible after scrolling past 50vh +- [ ] Button hidden on screens < 1280px +- [ ] Button visible on screens ≥ 1280px +- [ ] Click scrolls to top smoothly +- [ ] Keyboard Enter/Space scrolls to top +- [ ] Focus ring visible on tab navigation +- [ ] ARIA label present +- [ ] Respects reduced motion preferences +- [ ] 100% test coverage maintained diff --git a/specs/001-scroll-to-top/research.md b/specs/001-scroll-to-top/research.md new file mode 100644 index 0000000..b1f66fe --- /dev/null +++ b/specs/001-scroll-to-top/research.md @@ -0,0 +1,179 @@ +# Research: Scroll to Top Button + +**Date**: 2026-03-06 +**Feature**: Scroll to Top Button +**Branch**: 001-scroll-to-top + +## Research Topics Resolved + +### 1. Scroll Event Handling in React 19 + +**Decision**: Use passive event listener with `useSyncExternalStore` pattern + +**Rationale**: + +- The project already uses `useSyncExternalStore` for `useMediaQuery` hook +- Passive listeners improve scroll performance by signaling browser that `preventDefault()` won't be called +- Pattern matches existing codebase conventions + +**Implementation Pattern**: + +```typescript +const subscribe = useCallback((callback: () => void) => { + window.addEventListener('scroll', callback, { passive: true }); + return () => window.removeEventListener('scroll', callback); +}, []); +``` + +**Alternatives Considered**: + +- `useEffect` with manual event listener: Works but less idiomatic for React 19 +- Lodash throttle/debounce: Adds dependency, unnecessary for this use case +- `requestAnimationFrame`: Overkill for simple visibility toggle + +**References**: + +- React 19 `useSyncExternalStore` docs +- MDN: Passive event listeners + +--- + +### 2. Smooth Scroll Behavior + +**Decision**: Use native `window.scrollTo({ behavior: 'smooth' })` with `prefers-reduced-motion` fallback + +**Rationale**: + +- Native API is simple and has 95%+ browser support +- Respects user accessibility preferences automatically +- No external dependencies required + +**Implementation**: + +```typescript +const handleClick = () => { + const prefersReducedMotion = window.matchMedia( + '(prefers-reduced-motion: reduce)', + ).matches; + + window.scrollTo({ + top: 0, + behavior: prefersReducedMotion ? 'auto' : 'smooth', + }); +}; +``` + +**Browser Support**: + +- Chrome 61+, Firefox 36+, Safari 15+, Edge 79+ +- Fallback to instant scroll (`behavior: 'auto'`) works everywhere + +**Alternatives Considered**: + +- CSS `scroll-behavior: smooth`: Doesn't work with programmatic scroll +- Custom animation libraries: Unnecessary complexity for this use case + +**References**: + +- MDN: `Window.scrollTo()` +- MDN: `prefers-reduced-motion` media query + +--- + +### 3. WCAG 2.1 AA Button Requirements + +**Decision**: Implement full keyboard support, visible focus, and ARIA labeling + +**Requirements**: +| Requirement | Implementation | +| ----------- | -------------- | +| Keyboard accessible | `onClick` + `onKeyDown` (Enter/Space) | +| Visible focus indicator | Tailwind `focus:ring-2 focus:ring-blue-500` | +| Accessible name | `aria-label="Scroll to top"` | +| Sufficient color contrast | Match existing design system (gray/blue) | +| 44px minimum touch target | 40-48px diameter satisfies requirement | + +**Implementation**: + +```typescript + + ); +} diff --git a/src/components/ScrollToTop/ScrollToTop.types.ts b/src/components/ScrollToTop/ScrollToTop.types.ts new file mode 100644 index 0000000..c357adf --- /dev/null +++ b/src/components/ScrollToTop/ScrollToTop.types.ts @@ -0,0 +1,8 @@ +/** + * ScrollToTop component props. + * This component is self-contained with no external props. + */ +// eslint-disable-next-line @typescript-eslint/no-empty-object-type +export interface ScrollToTopProps { + // No props - component is self-contained +} diff --git a/src/components/ScrollToTop/index.ts b/src/components/ScrollToTop/index.ts new file mode 100644 index 0000000..e43d1e9 --- /dev/null +++ b/src/components/ScrollToTop/index.ts @@ -0,0 +1,2 @@ +export { ScrollToTop } from './ScrollToTop'; +export type { ScrollToTopProps } from './ScrollToTop.types'; diff --git a/src/hooks/useScrollPosition.test.ts b/src/hooks/useScrollPosition.test.ts new file mode 100644 index 0000000..a5b33c9 --- /dev/null +++ b/src/hooks/useScrollPosition.test.ts @@ -0,0 +1,176 @@ +import { act, renderHook } from '@testing-library/react'; + +import { useScrollPosition } from './useScrollPosition'; + +describe('useScrollPosition', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const originalScrollY: number | undefined = Object.getOwnPropertyDescriptor( + window, + 'scrollY', + )?.value; + + beforeEach(() => { + Object.defineProperty(window, 'scrollY', { + value: 0, + writable: true, + configurable: true, + }); + }); + + afterEach(() => { + if (originalScrollY !== undefined) { + Object.defineProperty(window, 'scrollY', { + value: originalScrollY, + writable: true, + configurable: true, + }); + } else { + Object.defineProperty(window, 'scrollY', { + value: 0, + writable: true, + configurable: true, + }); + } + }); + + it('returns false when at top of page with 50vh threshold', () => { + Object.defineProperty(window, 'scrollY', { + value: 0, + writable: true, + configurable: true, + }); + Object.defineProperty(window, 'innerHeight', { + value: 800, + writable: true, + configurable: true, + }); + + const { result } = renderHook(() => + useScrollPosition({ threshold: '50vh' }), + ); + + expect(result.current.isScrolledPastThreshold).toBe(false); + expect(result.current.scrollY).toBe(0); + }); + + it('returns true when scrolled past 50vh threshold', () => { + Object.defineProperty(window, 'scrollY', { + value: 500, + writable: true, + configurable: true, + }); + Object.defineProperty(window, 'innerHeight', { + value: 800, + writable: true, + configurable: true, + }); + + const { result } = renderHook(() => + useScrollPosition({ threshold: '50vh' }), + ); + + // 50vh = 400px, scrollY = 500, so should be past threshold + expect(result.current.isScrolledPastThreshold).toBe(true); + expect(result.current.scrollY).toBe(500); + }); + + it('returns false when scrolled but not past 50vh threshold', () => { + Object.defineProperty(window, 'scrollY', { + value: 300, + writable: true, + configurable: true, + }); + Object.defineProperty(window, 'innerHeight', { + value: 800, + writable: true, + configurable: true, + }); + + const { result } = renderHook(() => + useScrollPosition({ threshold: '50vh' }), + ); + + // 50vh = 400px, scrollY = 300, so should NOT be past threshold + expect(result.current.isScrolledPastThreshold).toBe(false); + expect(result.current.scrollY).toBe(300); + }); + + it('uses numeric threshold when provided', () => { + Object.defineProperty(window, 'scrollY', { + value: 250, + writable: true, + configurable: true, + }); + + const { result } = renderHook(() => useScrollPosition({ threshold: 200 })); + + // scrollY = 250, threshold = 200, so should be past threshold + expect(result.current.isScrolledPastThreshold).toBe(true); + expect(result.current.scrollY).toBe(250); + }); + + it('updates when scroll position changes', () => { + Object.defineProperty(window, 'innerHeight', { + value: 800, + writable: true, + configurable: true, + }); + Object.defineProperty(window, 'scrollY', { + value: 0, + writable: true, + configurable: true, + }); + + const { result } = renderHook(() => + useScrollPosition({ threshold: '50vh' }), + ); + + expect(result.current.isScrolledPastThreshold).toBe(false); + + act(() => { + Object.defineProperty(window, 'scrollY', { + value: 500, + writable: true, + configurable: true, + }); + window.dispatchEvent(new Event('scroll')); + }); + + expect(result.current.isScrolledPastThreshold).toBe(true); + expect(result.current.scrollY).toBe(500); + }); + + it('removes scroll event listener on unmount', () => { + const removeEventListenerSpy = vi.spyOn(window, 'removeEventListener'); + + const { unmount } = renderHook(() => + useScrollPosition({ threshold: '50vh' }), + ); + + unmount(); + + expect(removeEventListenerSpy).toHaveBeenCalledWith( + 'scroll', + expect.any(Function), + ); + removeEventListenerSpy.mockRestore(); + }); + + it('defaults to 50vh threshold when no options provided', () => { + Object.defineProperty(window, 'scrollY', { + value: 0, + writable: true, + configurable: true, + }); + Object.defineProperty(window, 'innerHeight', { + value: 800, + writable: true, + configurable: true, + }); + + const { result } = renderHook(() => useScrollPosition()); + + expect(result.current.scrollY).toBe(0); + expect(result.current.isScrolledPastThreshold).toBe(false); + }); +}); diff --git a/src/hooks/useScrollPosition.ts b/src/hooks/useScrollPosition.ts new file mode 100644 index 0000000..f92fc94 --- /dev/null +++ b/src/hooks/useScrollPosition.ts @@ -0,0 +1,77 @@ +import { useCallback, useRef, useSyncExternalStore } from 'react'; + +/** + * Options for useScrollPosition hook. + */ +export interface UseScrollPositionOptions { + /** Scroll threshold for visibility (e.g., '50vh' or numeric pixels) */ + threshold: number | '50vh'; +} + +/** + * Return value from useScrollPosition hook. + */ +export interface UseScrollPositionReturn { + /** Whether the page is scrolled past the threshold */ + isScrolledPastThreshold: boolean; + /** Current vertical scroll position in pixels */ + scrollY: number; +} + +/** + * Tracks scroll position and returns whether scrolled past a threshold. + * Uses passive event listener for performance. + */ +export function useScrollPosition( + options?: UseScrollPositionOptions, +): UseScrollPositionReturn { + const threshold = options?.threshold ?? '50vh'; + const snapshotRef = useRef<{ + scrollY: number; + isScrolledPastThreshold: boolean; + } | null>(null); + + const getThresholdInPixels = useCallback((): number => { + if (threshold === '50vh') { + return window.innerHeight / 2; + } + return threshold; + }, [threshold]); + + const subscribe = useCallback((callback: () => void) => { + window.addEventListener('scroll', callback, { passive: true }); + return (): void => { + window.removeEventListener('scroll', callback); + }; + }, []); + + const getSnapshot = useCallback(() => { + const scrollY = window.scrollY || window.pageYOffset; + const thresholdPx = getThresholdInPixels(); + const isScrolledPastThreshold = scrollY > thresholdPx; + + // Cache the snapshot to avoid infinite loops + if ( + snapshotRef.current?.scrollY !== scrollY || + snapshotRef.current.isScrolledPastThreshold !== isScrolledPastThreshold + ) { + snapshotRef.current = { scrollY, isScrolledPastThreshold }; + } + + return snapshotRef.current; + }, [getThresholdInPixels]); + + /* v8 ignore next -- @preserve */ + const getServerSnapshot = useCallback( + () => ({ scrollY: 0, isScrolledPastThreshold: false }), + [], + ); + + const { scrollY, isScrolledPastThreshold } = useSyncExternalStore( + subscribe, + getSnapshot, + getServerSnapshot, + ); + + return { scrollY, isScrolledPastThreshold }; +} From ff0d9b805ea1392f9eb8e928f09d7f5f92529777 Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 22:24:44 -0500 Subject: [PATCH 10/15] refactor: remove redundant handleKeyDown from ScrollToTop - Native From e09ccb676366bbafadc38ae3d6ed4120937c2a58 Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 22:49:14 -0500 Subject: [PATCH 12/15] fix(components): add cursor-pointer to toggle buttons - ViewToggle: unified and side-by-side buttons - DiffMethodToggle: characters, words, and lines buttons - Consistent with ScrollToTop button styling Co-authored-by: Qwen-Coder --- src/components/DiffMethodToggle/DiffMethodToggle.tsx | 6 +++--- src/components/ViewToggle/ViewToggle.tsx | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/DiffMethodToggle/DiffMethodToggle.tsx b/src/components/DiffMethodToggle/DiffMethodToggle.tsx index 1639e35..e58433e 100644 --- a/src/components/DiffMethodToggle/DiffMethodToggle.tsx +++ b/src/components/DiffMethodToggle/DiffMethodToggle.tsx @@ -11,7 +11,7 @@ export function DiffMethodToggle({ onClick={() => { onMethodChange('characters'); }} - className={`rounded-l-md px-3 py-1.5 text-sm font-medium transition-colors ${ + className={`cursor-pointer rounded-l-md px-3 py-1.5 text-sm font-medium transition-colors ${ activeMethod === 'characters' ? 'bg-blue-500 text-white dark:bg-blue-600' : 'bg-gray-100 text-gray-700 hover:bg-gray-200 dark:bg-gray-700 dark:text-gray-300 dark:hover:bg-gray-600' @@ -24,7 +24,7 @@ export function DiffMethodToggle({ onClick={() => { onMethodChange('words'); }} - className={`px-3 py-1.5 text-sm font-medium transition-colors ${ + className={`cursor-pointer px-3 py-1.5 text-sm font-medium transition-colors ${ activeMethod === 'words' ? 'bg-blue-500 text-white dark:bg-blue-600' : 'bg-gray-100 text-gray-700 hover:bg-gray-200 dark:bg-gray-700 dark:text-gray-300 dark:hover:bg-gray-600' @@ -37,7 +37,7 @@ export function DiffMethodToggle({ onClick={() => { onMethodChange('lines'); }} - className={`rounded-r-md px-3 py-1.5 text-sm font-medium transition-colors ${ + className={`cursor-pointer rounded-r-md px-3 py-1.5 text-sm font-medium transition-colors ${ activeMethod === 'lines' ? 'bg-blue-500 text-white dark:bg-blue-600' : 'bg-gray-100 text-gray-700 hover:bg-gray-200 dark:bg-gray-700 dark:text-gray-300 dark:hover:bg-gray-600' diff --git a/src/components/ViewToggle/ViewToggle.tsx b/src/components/ViewToggle/ViewToggle.tsx index 9a2e1c3..12cd231 100644 --- a/src/components/ViewToggle/ViewToggle.tsx +++ b/src/components/ViewToggle/ViewToggle.tsx @@ -12,7 +12,7 @@ export function ViewToggle({ activeMode, onModeChange }: ViewToggleProps) { onClick={() => { onModeChange('unified'); }} - className={`rounded-l-md px-3 py-1.5 text-sm font-medium transition-colors ${ + className={`cursor-pointer rounded-l-md px-3 py-1.5 text-sm font-medium transition-colors ${ activeMode === 'unified' ? 'bg-blue-500 text-white dark:bg-blue-600' : 'bg-gray-100 text-gray-700 hover:bg-gray-200 dark:bg-gray-700 dark:text-gray-300 dark:hover:bg-gray-600' @@ -25,7 +25,7 @@ export function ViewToggle({ activeMode, onModeChange }: ViewToggleProps) { onClick={() => { onModeChange('side-by-side'); }} - className={`rounded-r-md px-3 py-1.5 text-sm font-medium transition-colors ${ + className={`cursor-pointer rounded-r-md px-3 py-1.5 text-sm font-medium transition-colors ${ activeMode === 'side-by-side' ? 'bg-blue-500 text-white dark:bg-blue-600' : 'bg-gray-100 text-gray-700 hover:bg-gray-200 dark:bg-gray-700 dark:text-gray-300 dark:hover:bg-gray-600' From c2e4e03207efdbcf80efb115819d4c80776a9c00 Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 22:52:41 -0500 Subject: [PATCH 13/15] docs(specs): set status to completed --- specs/001-fix-diff-gutter/spec.md | 2 +- specs/001-fix-line-number-scrolling/data-model.md | 2 +- specs/001-fix-line-number-scrolling/quickstart.md | 2 +- specs/001-fix-line-number-scrolling/research.md | 2 +- specs/001-fix-line-number-scrolling/spec.md | 2 +- specs/001-scroll-to-top/spec.md | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/specs/001-fix-diff-gutter/spec.md b/specs/001-fix-diff-gutter/spec.md index 08ad8d5..c11eb86 100644 --- a/specs/001-fix-diff-gutter/spec.md +++ b/specs/001-fix-diff-gutter/spec.md @@ -2,7 +2,7 @@ **Feature Branch**: `001-fix-diff-gutter` **Created**: 2026-03-03 -**Status**: Implemented +**Status**: Completed **Input**: User description: "Fix line numbers in diff gutter" ## Clarifications diff --git a/specs/001-fix-line-number-scrolling/data-model.md b/specs/001-fix-line-number-scrolling/data-model.md index 81cec50..beed052 100644 --- a/specs/001-fix-line-number-scrolling/data-model.md +++ b/specs/001-fix-line-number-scrolling/data-model.md @@ -2,7 +2,7 @@ **Date**: 2026-02-26 **Feature**: Line number scrolling synchronization -**Status**: Complete +**Status**: Completed ## Core Entities diff --git a/specs/001-fix-line-number-scrolling/quickstart.md b/specs/001-fix-line-number-scrolling/quickstart.md index 88aed80..325d2df 100644 --- a/specs/001-fix-line-number-scrolling/quickstart.md +++ b/specs/001-fix-line-number-scrolling/quickstart.md @@ -2,7 +2,7 @@ **Branch**: `001-fix-line-number-scrolling` **Date**: 2026-02-26 -**Status**: Ready for implementation +**Status**: Completed ## Overview diff --git a/specs/001-fix-line-number-scrolling/research.md b/specs/001-fix-line-number-scrolling/research.md index 937b809..38036f8 100644 --- a/specs/001-fix-line-number-scrolling/research.md +++ b/specs/001-fix-line-number-scrolling/research.md @@ -2,7 +2,7 @@ **Date**: 2026-02-26 **Feature**: Line number scrolling synchronization -**Status**: Complete +**Status**: Completed ## Scroll Event Synchronization diff --git a/specs/001-fix-line-number-scrolling/spec.md b/specs/001-fix-line-number-scrolling/spec.md index 9b3c5a3..3ec29e9 100644 --- a/specs/001-fix-line-number-scrolling/spec.md +++ b/specs/001-fix-line-number-scrolling/spec.md @@ -2,7 +2,7 @@ **Feature Branch**: `001-fix-line-number-scrolling` **Created**: 2026-02-26 -**Status**: Completed (User Story 1 & 3) / Draft (User Story 2) +**Status**: Completed **Input**: User description: "fix line number scrolling" ## User Scenarios & Testing _(mandatory)_ diff --git a/specs/001-scroll-to-top/spec.md b/specs/001-scroll-to-top/spec.md index 9e094a5..4b65b09 100644 --- a/specs/001-scroll-to-top/spec.md +++ b/specs/001-scroll-to-top/spec.md @@ -2,7 +2,7 @@ **Feature Branch**: `001-scroll-to-top` **Created**: 2026-03-06 -**Status**: Draft +**Status**: Completed **Input**: User description: "scroll to top button fixed to bottom-right above XL breakpoint" ## Clarifications From c5c3e2b77099b0af9c9ebd270b39ab498ec2489b Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 22:59:06 -0500 Subject: [PATCH 14/15] refactor: remove unused type exports - Remove ScrollToTopProps interface (component has no props) - Remove UseScrollPositionOptions export (internal only) - Remove UseScrollPositionReturn export (internal only) Co-authored-by: Qwen-Coder --- src/components/ScrollToTop/ScrollToTop.types.ts | 8 -------- src/components/ScrollToTop/index.ts | 1 - src/hooks/useScrollPosition.ts | 10 ++-------- 3 files changed, 2 insertions(+), 17 deletions(-) delete mode 100644 src/components/ScrollToTop/ScrollToTop.types.ts diff --git a/src/components/ScrollToTop/ScrollToTop.types.ts b/src/components/ScrollToTop/ScrollToTop.types.ts deleted file mode 100644 index c357adf..0000000 --- a/src/components/ScrollToTop/ScrollToTop.types.ts +++ /dev/null @@ -1,8 +0,0 @@ -/** - * ScrollToTop component props. - * This component is self-contained with no external props. - */ -// eslint-disable-next-line @typescript-eslint/no-empty-object-type -export interface ScrollToTopProps { - // No props - component is self-contained -} diff --git a/src/components/ScrollToTop/index.ts b/src/components/ScrollToTop/index.ts index e43d1e9..17a27b5 100644 --- a/src/components/ScrollToTop/index.ts +++ b/src/components/ScrollToTop/index.ts @@ -1,2 +1 @@ export { ScrollToTop } from './ScrollToTop'; -export type { ScrollToTopProps } from './ScrollToTop.types'; diff --git a/src/hooks/useScrollPosition.ts b/src/hooks/useScrollPosition.ts index f92fc94..975da67 100644 --- a/src/hooks/useScrollPosition.ts +++ b/src/hooks/useScrollPosition.ts @@ -1,17 +1,11 @@ import { useCallback, useRef, useSyncExternalStore } from 'react'; -/** - * Options for useScrollPosition hook. - */ -export interface UseScrollPositionOptions { +interface UseScrollPositionOptions { /** Scroll threshold for visibility (e.g., '50vh' or numeric pixels) */ threshold: number | '50vh'; } -/** - * Return value from useScrollPosition hook. - */ -export interface UseScrollPositionReturn { +interface UseScrollPositionReturn { /** Whether the page is scrolled past the threshold */ isScrolledPastThreshold: boolean; /** Current vertical scroll position in pixels */ From fd5fd39cb371f6d840a1dfa3d4c31ff085e1fb4f Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 6 Mar 2026 23:14:59 -0500 Subject: [PATCH 15/15] test: remove duplicate and low-value tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - useScrollPosition: remove 3 redundant threshold tests, keep numeric threshold and default options tests for coverage - getDiffLineClasses: remove 2 repetitive CSS assertion tests - useDiff: remove 5 tests (redundant null checks, detects added/removed, unicode); consolidate lines array tests Result: 135 → 127 tests, maintains 100% coverage Co-authored-by: Qwen-Coder --- src/hooks/useDiff.test.ts | 44 ++-------------------- src/hooks/useScrollPosition.test.ts | 55 +++++++++------------------- src/utils/getDiffLineClasses.test.ts | 22 ----------- 3 files changed, 20 insertions(+), 101 deletions(-) diff --git a/src/hooks/useDiff.test.ts b/src/hooks/useDiff.test.ts index 26465b6..8a3de7b 100644 --- a/src/hooks/useDiff.test.ts +++ b/src/hooks/useDiff.test.ts @@ -8,16 +8,6 @@ describe('useDiff', () => { expect(result.current).toBeNull(); }); - it('returns null when original text is empty', () => { - const { result } = renderHook(() => useDiff('', 'some text')); - expect(result.current).toBeNull(); - }); - - it('returns null when modified text is empty', () => { - const { result } = renderHook(() => useDiff('some text', '')); - expect(result.current).toBeNull(); - }); - it('returns hasChanges false when texts are identical', () => { const { result } = renderHook(() => useDiff('hello world', 'hello world')); expect(result.current).not.toBeNull(); @@ -44,32 +34,6 @@ describe('useDiff', () => { expect(hasUnchanged).toBe(true); }); - it('detects added text correctly', () => { - const { result } = renderHook(() => useDiff('hello', 'hello world')); - expect(result.current?.hasChanges).toBe(true); - - const addedSegments = result.current?.segments.filter( - (s) => s.type === 'added', - ); - expect(addedSegments?.length).toBeGreaterThan(0); - }); - - it('detects removed text correctly', () => { - const { result } = renderHook(() => useDiff('hello world', 'hello')); - expect(result.current?.hasChanges).toBe(true); - - const removedSegments = result.current?.segments.filter( - (s) => s.type === 'removed', - ); - expect(removedSegments?.length).toBeGreaterThan(0); - }); - - it('handles special characters and unicode', () => { - const { result } = renderHook(() => useDiff('café ☕', 'café 🍵')); - expect(result.current).not.toBeNull(); - expect(result.current?.hasChanges).toBe(true); - }); - it('memoizes result for same inputs', () => { const { result, rerender } = renderHook( ({ original, modified }) => useDiff(original, modified), @@ -117,17 +81,15 @@ describe('useDiff', () => { ); }); - it('includes lines array in result', () => { + it('returns correct lines array with line numbers', () => { const { result } = renderHook(() => useDiff('hello world', 'hello world')); expect(result.current?.lines).toBeDefined(); expect(result.current?.lines.length).toBeGreaterThan(0); - }); - it('returns correct line numbers for line-level diff', () => { - const { result } = renderHook(() => + const { result: diffResult } = renderHook(() => useDiff('line1\nline2\n', 'line1\nchanged\n', 'lines'), ); - const lines = result.current?.lines ?? []; + const lines = diffResult.current?.lines ?? []; const unchanged = lines.filter((l) => l.type === 'unchanged'); expect(unchanged[0]).toMatchObject({ diff --git a/src/hooks/useScrollPosition.test.ts b/src/hooks/useScrollPosition.test.ts index a5b33c9..8c08ee9 100644 --- a/src/hooks/useScrollPosition.test.ts +++ b/src/hooks/useScrollPosition.test.ts @@ -33,7 +33,7 @@ describe('useScrollPosition', () => { } }); - it('returns false when at top of page with 50vh threshold', () => { + it('returns false when at top of page', () => { Object.defineProperty(window, 'scrollY', { value: 0, writable: true, @@ -53,7 +53,7 @@ describe('useScrollPosition', () => { expect(result.current.scrollY).toBe(0); }); - it('returns true when scrolled past 50vh threshold', () => { + it('returns true when scrolled past threshold', () => { Object.defineProperty(window, 'scrollY', { value: 500, writable: true, @@ -74,41 +74,6 @@ describe('useScrollPosition', () => { expect(result.current.scrollY).toBe(500); }); - it('returns false when scrolled but not past 50vh threshold', () => { - Object.defineProperty(window, 'scrollY', { - value: 300, - writable: true, - configurable: true, - }); - Object.defineProperty(window, 'innerHeight', { - value: 800, - writable: true, - configurable: true, - }); - - const { result } = renderHook(() => - useScrollPosition({ threshold: '50vh' }), - ); - - // 50vh = 400px, scrollY = 300, so should NOT be past threshold - expect(result.current.isScrolledPastThreshold).toBe(false); - expect(result.current.scrollY).toBe(300); - }); - - it('uses numeric threshold when provided', () => { - Object.defineProperty(window, 'scrollY', { - value: 250, - writable: true, - configurable: true, - }); - - const { result } = renderHook(() => useScrollPosition({ threshold: 200 })); - - // scrollY = 250, threshold = 200, so should be past threshold - expect(result.current.isScrolledPastThreshold).toBe(true); - expect(result.current.scrollY).toBe(250); - }); - it('updates when scroll position changes', () => { Object.defineProperty(window, 'innerHeight', { value: 800, @@ -156,7 +121,21 @@ describe('useScrollPosition', () => { removeEventListenerSpy.mockRestore(); }); - it('defaults to 50vh threshold when no options provided', () => { + it('uses numeric threshold when provided', () => { + Object.defineProperty(window, 'scrollY', { + value: 250, + writable: true, + configurable: true, + }); + + const { result } = renderHook(() => useScrollPosition({ threshold: 200 })); + + // scrollY = 250, threshold = 200, so should be past threshold + expect(result.current.isScrolledPastThreshold).toBe(true); + expect(result.current.scrollY).toBe(250); + }); + + it('defaults to 50vh when no options provided', () => { Object.defineProperty(window, 'scrollY', { value: 0, writable: true, diff --git a/src/utils/getDiffLineClasses.test.ts b/src/utils/getDiffLineClasses.test.ts index 5b57d16..6b13c55 100644 --- a/src/utils/getDiffLineClasses.test.ts +++ b/src/utils/getDiffLineClasses.test.ts @@ -23,28 +23,6 @@ describe('getDiffLineClasses', () => { ); }); - it('returns green classes for added lines', () => { - const result = getDiffLineClasses('added'); - - expect(result.lineNumberClasses).toBe( - 'w-8 px-2 text-right font-mono text-sm leading-6 text-gray-500 dark:text-gray-400 bg-green-50 dark:bg-green-900/20', - ); - expect(result.contentClasses).toBe( - 'pl-2 font-mono text-sm leading-6 dark:text-gray-100 bg-green-100 text-green-800 dark:bg-green-900/30 dark:text-green-300', - ); - }); - - it('returns red classes for removed lines', () => { - const result = getDiffLineClasses('removed'); - - expect(result.lineNumberClasses).toBe( - 'w-8 px-2 text-right font-mono text-sm leading-6 text-gray-500 dark:text-gray-400 bg-red-50 dark:bg-red-900/20', - ); - expect(result.contentClasses).toBe( - 'pl-2 font-mono text-sm leading-6 dark:text-gray-100 bg-red-100 text-red-800 dark:bg-red-900/30 dark:text-red-300', - ); - }); - it('uses custom content base classes when provided', () => { const customBase = 'min-w-0 flex-1 pl-2 font-mono text-sm leading-6 text-gray-900 dark:text-gray-100 whitespace-pre-wrap break-words';