[FEATURE] Interactive Issue Preview Card with Animations (#125)#239
Conversation
- Implement 3D flip animation for issue cards with front/back faces - Add IssueCardSkeleton component for loading states - Create back panel with detailed issue information and metadata - Add keyboard support (Enter/Space to flip, Escape to close) - Fix z-index management for flipped cards using CSS :has() selector - Convert FilterHelpTooltip to full-page centered modal - Convert FilterBuilder to full-page centered modal - Fix accessibility: change orphan label to span in SearchForm - Add GPU-optimized animations with will-change and backface-visibility - Implement smooth transitions with proper timing functions
Deploying issueflow with
|
| Latest commit: |
fb05d15
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://13a59f65.issueflow.pages.dev |
| Branch Preview URL: | https://feature-125-interactive-prev.issueflow.pages.dev |
|
Caution Review failedThe pull request is closed. Note
|
| Cohort / File(s) | Summary |
|---|---|
Issue Card Flip Feature src/components/results/IssueCard.svelte, src/components/results/ResultsContainer.svelte |
Implements card flip state/UI (front/back faces, flip button, screen-reader live region), body preview and copy/view actions in IssueCard; ResultsContainer supplies flip CSS (perspective, transforms, z-index), hover/focus visuals, reduced-motion pathways, and flip-button styling. |
Loading Skeleton src/components/results/IssueCardSkeleton.svelte, src/components/results/index.ts |
Adds IssueCardSkeleton component (mobile/desktop variants, shimmer, count & showLabels props) and exports it from the results index. |
Data Model & Utilities src/lib/github-graphql.ts, src/lib/issue-utils.ts, tests/issue-utils.test.ts |
Adds body: string to GitHubIssue and queries/maps it; introduces getBodyPreview(issue, maxLength) and hasBody(issue) utilities; test mocks updated with body. |
Filter & Help Modal Refactor src/components/shared/FilterBuilder.svelte, src/components/shared/FilterHelpTooltip.svelte |
Replaces inline add-filter menu and tooltip with modal/dialog overlays rendered at document body level; implements modal keyboard/backdrop handling, ARIA semantics, type/value flow, and focus/escape behavior. |
Minor Semantic Change src/components/results/SearchForm.svelte |
Replaced a label with a span for the Advanced Filters header (semantic-only change). |
Sequence Diagram(s)
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
- [FEATURE] Add loading skeleton cards during search #172: Adds IssueCardSkeleton and skeleton integration — aligns with the new
IssueCardSkeletoncomponent and exports. - [FEATURE] Add fade-in animation for issue cards #183: Modifies animations/transitions in
IssueCard.svelteandResultsContainer.svelte— related to the flip and hover/animation changes. - [FEATURE] Interactive Issue Preview Card with Animations #125: Feature request for interactive flip cards, hover animations, skeletons, and reduced-motion — this PR implements those objectives.
Possibly related PRs
- [FEATURE] Advanced Search Filters with Boolean Operators (#121) #237: Modal-based refactors for filter UI overlap with the
FilterBuilder.svelte/FilterHelpTooltip.sveltemodal changes here. - [FEATURE] Add copy issue link button to issue cards #14: Prior work on per-card copy/view state and
copiedIssueNumberinteracts with the copy actions preserved/used inIssueCard. - [REFACTOR] Decompose ResultsList.svelte into modular components #39: Earlier modifications to
IssueCard/ResultsContainertouch the same UI surface extended by this PR.
Pre-merge checks and finishing touches
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main feature: interactive issue preview cards with animations, relating directly to the core changes in the changeset. |
| Description check | ✅ Passed | The PR description provides a comprehensive summary of changes, includes the required test plan, and references issue #125. Although the full template checklist is not explicitly marked, the description covers the essential elements: summary, type of change (new feature), testing details, and additional context. |
| Linked Issues check | ✅ Passed | All acceptance criteria from issue #125 are met: hover animation effects implemented [ResultsContainer.svelte], card flip animation implemented [IssueCard.svelte, ResultsContainer.svelte], skeleton animations added [IssueCardSkeleton.svelte], and reduced-motion preferences respected [ResultsContainer.svelte]. |
| Out of Scope Changes check | ✅ Passed | All changes are directly scoped to issue #125 requirements. Modifications to FilterHelpTooltip, FilterBuilder, and SearchForm support improved UX/accessibility alongside the card flip feature without introducing unrelated functionality. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/shared/FilterBuilder.sveltesrc/components/shared/FilterHelpTooltip.svelte
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/components/shared/FilterHelpTooltip.svelte (2)
107-114: Modal implementation looks good, but consider removing redundant keydown handler.The
onkeydown={handleKeydown}on the backdrop div (line 113) is redundant since Escape handling is already registered at the document level via$effect(lines 66-75). Both will fire when Escape is pressed.Additionally, this modal lacks focus trapping—users can Tab outside the modal to background elements. For a help modal this is low risk, but for full WCAG 2.4.3 compliance, consider trapping focus.
🔎 Remove redundant keydown handler
<div class="fixed inset-0 bg-black/70 backdrop-blur-sm z-50 flex items-center justify-center p-4" onclick={closeTooltip} - onkeydown={handleKeydown} >
78-105: LGTM with minor ARIA suggestion.The trigger button accessibility is well-implemented with
aria-expandedandaria-label. However,aria-describedby(line 87) typically describes the element itself, not what it controls. For a button that opens a dialog,aria-controlsis more semantically appropriate.🔎 Consider using aria-controls instead
onclick={toggleTooltip} - aria-describedby={show ? tooltipId : undefined} + aria-controls={tooltipId} aria-expanded={show}src/components/shared/FilterBuilder.svelte (2)
139-167:handleClickOutsideis now dead code with the modal pattern.With the new full-screen modal overlay, clicks outside the modal content hit the backdrop, which directly calls
resetAddFormviaonclick={resetAddForm}(line 249). ThehandleClickOutsidefunction and its document event listener registration (lines 159-167) are no longer reached and can be removed.🔎 Remove dead code
- /** - * Handle click outside to close menu - */ - function handleClickOutside(event: MouseEvent): void { - if (addMenuRef && !addMenuRef.contains(event.target as Node)) { - if (showAddMenu && !selectedFilterType) { - resetAddForm(); - } - } - } // ... other code ... - // Close menu on outside click - $effect(() => { - if (showAddMenu) { - document.addEventListener('click', handleClickOutside); - return () => { - document.removeEventListener('click', handleClickOutside); - }; - } - });
279-291: Consider removingrole="menuitem"from type buttons.If changing the container from
role="menu"torole="dialog"(as suggested above), therole="menuitem"on these buttons becomes invalid and should be removed. Regular buttons inside a dialog don't need special roles.🔎 Remove role="menuitem"
<button type="button" class="type-btn px-2 py-1 rounded text-xs bg-slate-700 text-slate-300 border border-slate-600 hover:bg-slate-600 hover:text-white focus:outline-none focus:ring-1 focus:ring-teal-500 transition-colors" onclick={() => selectFilterType(type)} - role="menuitem" >
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/results/IssueCard.sveltesrc/components/results/IssueCardSkeleton.sveltesrc/components/results/ResultsContainer.sveltesrc/components/results/SearchForm.sveltesrc/components/results/index.tssrc/components/shared/FilterBuilder.sveltesrc/components/shared/FilterHelpTooltip.sveltesrc/lib/github-graphql.tssrc/lib/issue-utils.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain TypeScript strict mode across all TypeScript files
Files:
src/lib/github-graphql.tssrc/lib/issue-utils.tssrc/components/results/index.ts
**/*.{svelte,astro,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use UnoCSS with Tailwind-compatible syntax for styling
Files:
src/lib/github-graphql.tssrc/lib/issue-utils.tssrc/components/results/IssueCardSkeleton.sveltesrc/components/results/index.tssrc/components/results/SearchForm.sveltesrc/components/shared/FilterBuilder.sveltesrc/components/results/IssueCard.sveltesrc/components/shared/FilterHelpTooltip.sveltesrc/components/results/ResultsContainer.svelte
**/*.{js,ts,tsx,svelte,astro}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure ESLint linting passes with 0 errors (run
npm run lint)
Files:
src/lib/github-graphql.tssrc/lib/issue-utils.tssrc/components/results/IssueCardSkeleton.sveltesrc/components/results/index.tssrc/components/results/SearchForm.sveltesrc/components/shared/FilterBuilder.sveltesrc/components/results/IssueCard.sveltesrc/components/shared/FilterHelpTooltip.sveltesrc/components/results/ResultsContainer.svelte
**/*.{js,ts,tsx,svelte,astro,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Format all code with Prettier including Svelte/Astro plugins (run
npm run format)
Files:
src/lib/github-graphql.tssrc/lib/issue-utils.tssrc/components/results/IssueCardSkeleton.sveltesrc/components/results/index.tssrc/components/results/SearchForm.sveltesrc/components/shared/FilterBuilder.sveltesrc/components/results/IssueCard.sveltesrc/components/shared/FilterHelpTooltip.sveltesrc/components/results/ResultsContainer.svelte
src/lib/github-graphql.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use GitHub GraphQL API for issue discovery as implemented in
src/lib/github-graphql.ts
Files:
src/lib/github-graphql.ts
**/*.svelte
📄 CodeRabbit inference engine (CLAUDE.md)
Use Svelte 5 runes ($state, $derived, $effect) for reactive state management
Files:
src/components/results/IssueCardSkeleton.sveltesrc/components/results/SearchForm.sveltesrc/components/shared/FilterBuilder.sveltesrc/components/results/IssueCard.sveltesrc/components/shared/FilterHelpTooltip.sveltesrc/components/results/ResultsContainer.svelte
🧠 Learnings (1)
📚 Learning: 2025-12-27T04:07:21.626Z
Learnt from: CR
Repo: VibeTensor/IssueFlow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-27T04:07:21.626Z
Learning: Applies to src/lib/github-graphql.ts : Use GitHub GraphQL API for issue discovery as implemented in `src/lib/github-graphql.ts`
Applied to files:
src/lib/github-graphql.tssrc/lib/issue-utils.tssrc/components/results/IssueCard.svelte
🧬 Code graph analysis (1)
src/lib/issue-utils.ts (1)
src/lib/github-graphql.ts (1)
GitHubIssue(15-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (17)
src/components/results/SearchForm.svelte (1)
515-532: Good accessibility fix.Changing the orphan
<label>to a<span>is correct. The previous<label>had no associated form control (noforattribute, no wrapped input), which is invalid HTML and confusing for screen readers. The<span>maintains the visual appearance while being semantically appropriate for a section header.src/lib/issue-utils.ts (2)
321-363: Well-implemented body preview with good markdown stripping.The implementation handles common markdown elements effectively, and the word-boundary truncation at 70% threshold is a nice touch to avoid awkward cuts.
One minor edge case: bodies consisting entirely of code blocks will result in just
[code]as the preview. This is acceptable for a preview feature.
365-373: Clean utility function.The
hasBodyfunction is concise and correctly uses optional chaining withBoolean()for a clean boolean return.src/lib/github-graphql.ts (2)
15-46: Body field addition is well-integrated.The
body: stringfield is correctly added to the interface, and the GraphQL query includes it. The type consistency is maintained across the codebase.
330-347: REST API mapping correctly defaults body to empty string.Using
issue.body || ''(line 335) ensures the field is always a string, preventing null/undefined propagation to consumers likegetBodyPreviewandhasBody.src/components/results/index.ts (1)
1-34: Clean barrel export addition.The
IssueCardSkeletonexport follows the established pattern, and the documentation comments are updated to reflect the new component in the hierarchy.src/components/results/IssueCardSkeleton.svelte (2)
10-22: Good use of Svelte 5 runes for props and derived state.Using
$props()and$derivedfollows the coding guidelines for Svelte 5 reactive state management. The skeleton array generation is clean and efficient.
24-101: Well-structured skeleton with proper accessibility.The skeleton correctly uses
aria-hidden="true"androle="presentation"to hide from screen readers—this is the right pattern for decorative loading indicators. The responsive layout (mobile/desktop variants) mirrors the actualIssueCardstructure for smooth visual transitions when content loads.src/components/results/IssueCard.svelte (4)
86-105: Minor: Flip button aria-label on front face uses inverse logic.The flip button on the front face (line 92) has
aria-label={isFlipped ? 'Show issue front' : 'Show issue details'}. Since this button is on the front face (visible when!isFlipped), the label will always be "Show issue details" when the button is visible, which is correct. The ternary handles the case where the component re-renders while animating. The implementation is fine.
346-406: Back face implementation is well-structured.The back face properly displays the body preview with
line-clamp-5for overflow handling, includes navigation back to front, and provides the "Read Full Issue" link. The comment count context helps users understand engagement level.
64-75: Excellent accessibility for flip state announcements.The screen reader announcement with
aria-live="polite"andaria-atomic="true"properly notifies assistive technology users when the card flips. Conditionally rendering only whenissueHasBodyprevents unnecessary announcements for non-flippable cards.
77-85: Flip animation CSS classes are properly defined in ResultsContainer.svelte.All required classes (
card-flip-container,card-flip-inner,card-front,card-back, andflip-button) are defined with the necessary 3D transform properties (perspective, transform-style: preserve-3d, rotateY transforms) and GPU optimizations (will-change on hover/focus-within, backface-visibility with vendor prefixes, and :has() selector for z-index control).src/components/results/ResultsContainer.svelte (5)
956-962: Excellent will-change optimization!Applying
will-change: transformonly during hover and focus-within interactions (rather than persistently) is a performance best practice. This avoids unnecessary GPU memory allocation while still optimizing for the flip animation when needed.
971-972: Good cross-browser backface-visibility implementation.Using both the
-webkit-prefixed and standardbackface-visibilityproperties ensures GPU acceleration for the flip animation works across different browser engines. This will provide smooth 3D transforms.
987-1021: Strong accessibility implementation for flip button.The flip button includes proper ARIA semantics (
aria-pressed), clear focus indicators (focus-visible), visual feedback states (hover, active), and appropriate keyboard support. This ensures the flip interaction is accessible to all users.
1284-1307: Excellent reduced-motion accessibility implementation.The reduced-motion media query properly disables decorative animations, removes transitions from flip elements, and simplifies the hover effect while maintaining functionality. The instant flip (Line 1306) without animation is the correct approach for users who prefer reduced motion.
947-949: No action required: :has() selector is appropriate for this project's scope.The
:has()pseudo-class is well-supported in all modern browsers and aligns with this project's target environment. Astro 5, Svelte 5, and the Node >= 18.0 requirement indicate a modern browser baseline. Even if unsupported in rare cases, the z-index enhancement gracefully degrades—flipped cards continue functioning, just without the stacking optimization. No fallback is needed.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/issue-utils.test.ts (1)
8-17: Add test coverage forgetBodyPreviewandhasBodyutilities.The utilities
getBodyPreviewandhasBodyare exported fromsrc/lib/issue-utils.tsbut lack corresponding tests in this file. Per coding guidelines, unit tests should be written for all utilities using Vitest. The existing mock issue helper already includes abodyproperty, making it straightforward to implement these tests.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/issue-utils.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain TypeScript strict mode across all TypeScript files
Files:
tests/issue-utils.test.ts
**/*.{svelte,astro,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use UnoCSS with Tailwind-compatible syntax for styling
Files:
tests/issue-utils.test.ts
**/*.{js,ts,tsx,svelte,astro}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure ESLint linting passes with 0 errors (run
npm run lint)
Files:
tests/issue-utils.test.ts
**/*.{js,ts,tsx,svelte,astro,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Format all code with Prettier including Svelte/Astro plugins (run
npm run format)
Files:
tests/issue-utils.test.ts
tests/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests for utilities and API functions using Vitest
Files:
tests/issue-utils.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-27T04:07:21.626Z
Learnt from: CR
Repo: VibeTensor/IssueFlow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-27T04:07:21.626Z
Learning: Applies to src/lib/github-graphql.ts : Use GitHub GraphQL API for issue discovery as implemented in `src/lib/github-graphql.ts`
Applied to files:
tests/issue-utils.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
tests/issue-utils.test.ts (1)
30-30: LGTM: Body field correctly added to mock.The
bodyproperty aligns the mock with the updatedGitHubIssuetype and supports the new body-related utilities introduced in this PR.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/shared/FilterBuilder.svelte (2)
244-260: Modal lacks focus management and focus trap.The modal opens without moving focus to the dialog, and there's no focus trap to prevent users from tabbing to elements behind the backdrop. Additionally,
onkeydownon the backdrop div (line 250) won't reliably receive keyboard events since the div isn't focusable.Consider:
- Move focus to the first focusable element (e.g., NOT button) when modal opens using an
$effect.- Move
onkeydownto the dialog element (line 252) or addtabindex="-1"to the dialog and focus it.- Optionally implement a focus trap for full modal accessibility.
🔎 Proposed fix for focus management
let valueInputRef = $state<HTMLInputElement | null>(null); + let dialogRef = $state<HTMLDivElement | null>(null); + // Focus management for modal + $effect(() => { + if (showAddMenu && dialogRef) { + dialogRef.focus(); + } + });Then update the dialog element:
<div + bind:this={dialogRef} class="add-filter-menu flex flex-wrap items-center gap-2 p-3 rounded-lg bg-slate-800 border border-slate-600 shadow-2xl shadow-black/50 max-w-md w-full" role="dialog" aria-modal="true" aria-label="Add filter" + tabindex="-1" onclick={(e) => e.stopPropagation()} + onkeydown={handleAddMenuKeydown} >And remove
onkeydownfrom the backdrop (line 250).
159-167: Consider removing redundant click-outside handler.The
handleClickOutsideeffect and function (lines 139-145, 159-167) check againstaddMenuRefwhich is bound to the button container, not the modal. Since the modal now handles click-outside via the backdrop'sonclick={resetAddForm}(line 249), this document-level listener appears to be leftover from the previous inline menu implementation.Additionally, the condition
showAddMenu && !selectedFilterType(line 141) would only close the modal if no filter type is selected, which is inconsistent with the backdrop click behavior that always closes.🔎 Proposed cleanup
- /** - * Handle click outside to close menu - */ - function handleClickOutside(event: MouseEvent): void { - if (addMenuRef && !addMenuRef.contains(event.target as Node)) { - if (showAddMenu && !selectedFilterType) { - resetAddForm(); - } - } - } // ... other code ... - // Close menu on outside click - $effect(() => { - if (showAddMenu) { - document.addEventListener('click', handleClickOutside); - return () => { - document.removeEventListener('click', handleClickOutside); - }; - } - });The
addMenuRefbinding (line 212) can also be removed if no longer needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/shared/FilterBuilder.svelte
🧰 Additional context used
📓 Path-based instructions (4)
**/*.svelte
📄 CodeRabbit inference engine (CLAUDE.md)
Use Svelte 5 runes ($state, $derived, $effect) for reactive state management
Files:
src/components/shared/FilterBuilder.svelte
**/*.{svelte,astro,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use UnoCSS with Tailwind-compatible syntax for styling
Files:
src/components/shared/FilterBuilder.svelte
**/*.{js,ts,tsx,svelte,astro}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure ESLint linting passes with 0 errors (run
npm run lint)
Files:
src/components/shared/FilterBuilder.svelte
**/*.{js,ts,tsx,svelte,astro,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Format all code with Prettier including Svelte/Astro plugins (run
npm run format)
Files:
src/components/shared/FilterBuilder.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
src/components/shared/FilterBuilder.svelte (3)
211-242: LGTM!The add filter button implementation is clean with proper disabled state handling and accessible SVG markup.
262-335: LGTM!The modal content implementation is well-structured with proper accessibility attributes (
aria-pressedon the NOT toggle), dynamic placeholders for context-aware input hints, and appropriate disabled state handling on the Add button.
337-361: LGTM!Cancel button has proper accessibility with
aria-labeland hidden decorative SVG.
1 participant
Summary
Test Plan
Closes #125
Summary by CodeRabbit
New Features
UX Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.