feat!: migrate to Vite + React + shadcn/ui single-page app#92
Conversation
Replace generic dark dev portfolio with distinctive Jon Bogaty personal brand. Research-backed design system: warm amber (#E8A849) primary, Instrument Serif headings, warm-tinted typography, all WCAG AAA verified. Design changes: - Cyan #06b6d4 → Amber #E8A849 (split-complementary with steel blue) - Space Grotesk → Instrument Serif (authority + contemporary edge) - Blue-white text → Warm-tinted white #F0EDE8 - All token names standardized (--primary → --brand, --bg-paper → --bg-surface) New pages: Journey (CSS-only tabs + timeline), Projects (curated showcase), Contact (social cards + resume downloads) Redesigned: Home (hero + stats + bento grid + CTA), Nav (JB wordmark), Footer (personal brand tagline) Removed: About page (split into Home/Journey), Ecosystem page (replaced by curated Projects page — ecosystem sites handle deep catalogs) Updated: Resume tokens, print/PDF colors (#B8862B for paper), favicon Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe project undergoes a comprehensive architecture migration from Astro (static site generator) to Vite (build tool) with React (UI framework). This includes replacing all Astro pages, components, and configuration with React alternatives, refactoring CI/CD workflows, introducing resume PDF/DOCX generation scripts, adding shadcn/ui components, and adopting Tailwind CSS for styling. The application transforms from a static multi-page site to a dynamic single-page application. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @jbdevprimary, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a comprehensive brand overhaul for the personal website, transforming its visual identity and content structure. The primary goal is to shift from a generic developer portfolio to a distinctive personal brand, emphasizing the author's senior engineering leadership and AI innovation. This involves a complete redesign of the color palette, typography, and site layout, introducing new pages for career journey, projects, and contact, while consolidating and removing outdated sections to create a more focused and impactful online presence. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Summary
Comprehensive personal brand overhaul successfully implemented with no blocking issues identified. The PR delivers exactly what's described: a complete design system refresh transitioning from cyan/Space Grotesk to warm amber/Instrument Serif, new Journey/Projects/Contact pages, redesigned Home page, and updated navigation/footer components.
Code Quality Assessment
Strengths:
- Zero client-side JavaScript maintained throughout (CSS-only tabs, hamburger menu, details/summary)
- Proper TypeScript typing and Astro component structure
- Excellent accessibility with ARIA labels, semantic HTML, and keyboard navigation
- Secure external link handling with
rel="noopener noreferrer" - Responsive design with well-structured media queries
- Clean CSS custom property system for theming
- Proper use of CSS Grid and Flexbox for layouts
Technical Implementation:
- CSS-only radio button tab system on Journey page works correctly
- Hamburger menu implementation using checkbox hack is sound
- Gradient text effects with proper fallbacks
- Color contrast considerations documented in design docs
- Proper HTML5 semantic structure throughout
The codebase is production-ready with no security vulnerabilities, logic errors, or blocking defects identified.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
There was a problem hiding this comment.
Code Review
This is a comprehensive and impressive pull request that successfully executes a major brand overhaul. The changes are extensive, covering the design system, typography, page structure, and content, all while adhering to a zero-JS philosophy. The new design is cohesive and aligns well with the stated goals. My review includes a few suggestions to enhance accessibility and code maintainability.
| | `--brand-dim` | `rgba(232,168,73,0.15)` | Tinted backgrounds | — | — | | ||
| | `--text-primary` | `#F0EDE8` | Headings, primary | 16.62:1 | AAA | | ||
| | `--text-secondary` | `#9A9690` | Body, descriptions | 6.60:1 | AA | | ||
| | `--text-disabled` | `#5A5750` | Muted elements | 3.18:1 | AA-large | |
There was a problem hiding this comment.
The contrast ratio of 3.18:1 for --text-disabled only meets WCAG AA for large text (at least 18pt or 14pt bold). If this color is used for smaller, muted body text, it may not be sufficiently accessible for users with visual impairments. Consider either restricting its use to large text only or adjusting the color to meet the 4.5:1 contrast ratio for normal text.
| { label: 'GitHub', href: 'https://github.com/jbcom', ariaLabel: 'GitHub profile' }, | ||
| { label: 'LinkedIn', href: 'https://linkedin.com/in/jonbogaty', ariaLabel: 'LinkedIn profile' }, |
There was a problem hiding this comment.
For better accessibility, the ariaLabel attributes for social links could be more descriptive. Instead of a generic 'GitHub profile', specifying whose profile it is (e.g., "Jon Bogaty's GitHub profile") provides clearer context for screen reader users.
| { label: 'GitHub', href: 'https://github.com/jbcom', ariaLabel: 'GitHub profile' }, | |
| { label: 'LinkedIn', href: 'https://linkedin.com/in/jonbogaty', ariaLabel: 'LinkedIn profile' }, | |
| { label: 'GitHub', href: 'https://github.com/jbcom', ariaLabel: "Jon Bogaty's GitHub profile" }, | |
| { label: 'LinkedIn', href: 'https://linkedin.com/in/jonbogaty', ariaLabel: "Jon Bogaty's LinkedIn profile" }, |
| .chip-typescript { | ||
| background: rgba(49, 120, 198, 0.15); | ||
| color: var(--lang-typescript); | ||
| color: #3178c6; | ||
| } |
There was a problem hiding this comment.
For better maintainability, you could define the language color once and reuse it for both the color and background properties. This avoids defining the color in two different formats (hex for color and RGB components for rgba background).
A modern CSS approach for this would be to use a variable for the RGB components:
:root {
/* in addition to other variables */
--color-lang-ts-rgb: 49, 120, 198;
}
.chip-typescript {
background: rgba(var(--color-lang-ts-rgb), 0.15);
color: rgb(var(--color-lang-ts-rgb));
}This keeps the color definition in one place per language, making future updates easier.
Remove old Vite/React/MUI files that were already deleted in working tree from the Astro migration but not yet committed (src/App.tsx, src/main.tsx, src/theme.ts, etc.). Also removes old static HTML files, old workflows, and old content files. Fixes CTA "Contact" button to link to /contact/ page instead of mailto. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address PR review feedback: make aria-labels more descriptive by including the person's name for better screen reader context. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…land - Add React + motion (framer-motion) via Astro islands for interactive timeline - Scroll-linked horizontal card animation with nav dots and progress bar - Split Flipside Crypto into 3 phases: DevOps culture, AI Operations, IT/Security - Source all career milestones and dates from resume.json - Fix homepage stats: replace hardcoded values with dynamic resume data - Expandable key highlights per timeline card - Responsive card sizing (380/320/280px breakpoints) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace Astro SSG multi-page site with a single-page Vite + React application using shadcn/ui components. The site is now one immersive page with two tabs (Career Timeline / Open-Source Projects) plus a contact footer. Architecture: - Vite 7.3 + React 19 + Tailwind CSS v4 + shadcn/ui (new-york) - Horizontal scroll career timeline with snap, mouse drag, touch swipe - Gradient fade scroll indicators + animated swipe hint on mobile - Responsive cards (viewport-adaptive on mobile, fixed on desktop) - Three monorepo project showcase (agentic.coach, extended-data.dev, strata.game) CI/CD (all actions SHA-pinned): - ci.yml: PR-only validation (lint, typecheck, build, test) - cd.yml: release-please on push to main - release.yml: deploy to GitHub Pages on release published - automerge.yml: auto-merge dependabot + release-please PRs Removes all Astro pages, layouts, components, and styles. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| name: Lint | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| task: | ||
| - name: Lint | ||
| command: pnpm lint | ||
| needs-chromium: false | ||
| - name: Typecheck | ||
| command: pnpm check | ||
| needs-chromium: false | ||
| - name: Build | ||
| command: pnpm build | ||
| needs-chromium: true | ||
| - name: Test | ||
| command: pnpm test --passWithNoTests || true | ||
| needs-chromium: false | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
||
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@9fd676a19091d4595eefd76e4bd31c97133911f1 # v4.2.0 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 | ||
| with: | ||
| node-version: 22 | ||
| cache: pnpm | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Lint | ||
| run: pnpm lint | ||
|
|
||
| typecheck: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, the fix is to define an explicit permissions block that grants only the minimal access the workflow requires. Since these jobs only need to read the repository contents (for actions/checkout) and upload artifacts (which does not require repo write access), the minimal permissions are contents: read. Because all jobs share the same needs, adding a single permissions block at the top level of the workflow (alongside on: and concurrency:) is the cleanest solution.
Concretely, in .github/workflows/ci.yml, add:
permissions:
contents: readbetween the on: block (lines 3–5) and the concurrency: block (lines 7–9). This will apply to all jobs (lint, typecheck, build, test) since none of them define their own permissions. No other changes, imports, or definitions are required, and existing functionality of the workflow remains unchanged while the GITHUB_TOKEN is now restricted.
| @@ -4,6 +4,9 @@ | ||
| pull_request: | ||
| branches: [main] | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| concurrency: | ||
| group: ci-${{ github.ref }} | ||
| cancel-in-progress: true |
| name: Typecheck | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
||
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| uses: pnpm/action-setup@9fd676a19091d4595eefd76e4bd31c97133911f1 # v4.2.0 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 | ||
| with: | ||
| node-version: 22 | ||
| cache: pnpm | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Install Chromium for Puppeteer | ||
| if: matrix.task.needs-chromium | ||
| run: npx puppeteer browsers install chrome | ||
| - name: Typecheck | ||
| run: pnpm check | ||
|
|
||
| - name: ${{ matrix.task.name }} | ||
| run: ${{ matrix.task.command }} | ||
| build: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
Generally, the problem is fixed by explicitly specifying a minimal permissions block so the GITHUB_TOKEN has only the rights required by the workflow. For a CI pipeline that just checks out code, installs dependencies, builds, lints, typechecks, tests, and uploads artifacts, contents: read is typically sufficient. No job appears to need write access to the repository or other resources.
The single best fix here is to add a root-level permissions block just under the workflow name (or under on:) so it applies to all jobs, and restrict it to contents: read. None of the steps use features (e.g., id-token, pull-requests, or packages) that would require additional permissions. Concretely, in .github/workflows/ci.yml, between name: CI (line 1) and on: (line 3), insert:
permissions:
contents: readNo extra imports or definitions are needed; this is pure YAML configuration inside the GitHub Actions workflow.
| @@ -1,5 +1,8 @@ | ||
| name: CI | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [main] | ||
| @@ -7,7 +10,6 @@ | ||
| concurrency: | ||
| group: ci-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| lint: | ||
| name: Lint |
| name: Build | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
||
| - name: Verify generated files | ||
| if: matrix.task.name == 'Build' | ||
| run: | | ||
| echo "Checking for generated resume files..." | ||
| ls -la dist/Jon_Bogaty_Resume.pdf dist/Jon_Bogaty_Resume.docx | ||
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@9fd676a19091d4595eefd76e4bd31c97133911f1 # v4.2.0 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 | ||
| with: | ||
| node-version: 22 | ||
| cache: pnpm | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Build | ||
| run: pnpm build | ||
|
|
||
| - name: Upload build artifact | ||
| if: matrix.task.name == 'Build' | ||
| uses: actions/upload-artifact@v4 | ||
| uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 | ||
| with: | ||
| name: dist | ||
| path: dist/ | ||
| retention-days: 7 | ||
|
|
||
| test: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, fix this by explicitly specifying permissions for the workflow (or each job) so that the GITHUB_TOKEN has only the scopes required. For a standard CI workflow that just checks out code, installs dependencies, runs lint/typecheck/tests, builds, and uploads artifacts, the minimal required permission is usually contents: read. No steps here need to write to the repo, issues, or PRs.
The single best fix without changing functionality is to add a top-level permissions: block right under the name: CI (and before on:) that sets contents: read. This applies to all jobs (lint, typecheck, build, test) because none of them define their own permissions. No imports or extra definitions are needed; this is just YAML configuration. Only .github/workflows/ci.yml needs to be updated.
| @@ -1,5 +1,8 @@ | ||
| name: CI | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [main] |
…sume generation pipeline
- Fix vitest.config.ts: replace Astro import with standard vitest config
- Fix env.d.ts: Vite client types instead of Astro
- Fix playwright.config.ts: port 4321 → 4173 (Vite)
- Fix biome.json: remove Astro overrides, exclude CSS (Tailwind v4)
- Fix App.tsx: name-based work entry lookups instead of fragile index access
- Fix CareerTimeline.tsx: rAF-throttled scroll, semantic <section> element
- Fix CI: remove || true from test step, add Playwright install for PDF gen
- Fix separator.tsx: remove Next.js 'use client' directive
- Remove all Astro artifacts: enterprise-branding/, old plan docs, .gitignore section
- Update ENTERPRISE.md: remove Astro/Starlight references
- Delete dead code: docx-builder.ts (replaced by generation pipeline)
- Add resume generation pipeline: scripts/generate-resume-{docx,pdf}.ts
- DOCX via `docx` npm package from resume.json
- PDF via Playwright rendering print-optimized HTML
- Wired as prebuild step: runs automatically before vite build
- Update release.yml: upload PDF/DOCX as release assets
- Rewrite E2E tests for SPA architecture
- Add noscript fallback to index.html
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| name: Test | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
||
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@9fd676a19091d4595eefd76e4bd31c97133911f1 # v4.2.0 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 | ||
| with: | ||
| node-version: 22 | ||
| cache: pnpm | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Test | ||
| run: pnpm test |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, the problem is fixed by explicitly setting a permissions block to restrict the GITHUB_TOKEN to the least privileges needed. For this CI workflow, all jobs use actions/checkout (requires contents: read) and actions/upload-artifact (does not need repo write access). No step writes to the repo, issues, or PRs, so we can safely set workflow-level permissions to read-only.
The single best fix with no functional change is to add a root-level permissions block, just after the on: or concurrency: section, applying to all jobs. We should grant contents: read (and optionally no other scopes). That satisfies CodeQL’s recommendation and documents the workflow’s needs. No other files or lines need modification, and no imports or additional definitions are required.
Concretely, in .github/workflows/ci.yml, insert:
permissions:
contents: readbetween the concurrency block and jobs: (or equivalently between on: and concurrency:); this will cover lint, typecheck, build, and test without further per-job changes.
| @@ -8,6 +8,9 @@ | ||
| group: ci-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| lint: | ||
| name: Lint |
- CRITICAL: escape all HTML interpolations in PDF generator (XSS prevention) - CRITICAL: add try/finally for Playwright browser lifecycle (resource cleanup) - MAJOR: extract shared formatDate/formatDateRange to src/lib/dates.ts (DRY) - MAJOR: add scripts/** to biome.json includes (lint coverage) - MAJOR: add scripts to tsconfig.json include (typecheck coverage) - MAJOR: add resume file verification step in release.yml - MAJOR: remove Google Fonts network dependency for deterministic PDF builds - Dismiss stale lodash vulnerability (no longer a dependency after Astro removal) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
12-108:⚠️ Potential issue | 🟠 MajorSet explicit least-privilege
GITHUB_TOKENpermissions for CI.This workflow currently relies on default token permissions. Add an explicit permissions block (at minimum
contents: read) to avoid over-privileged CI tokens.Suggested fix
on: pull_request: branches: [main] +permissions: + contents: read + concurrency: group: ci-${{ github.ref }} cancel-in-progress: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 12 - 108, The workflow currently uses default GITHUB_TOKEN permissions; add an explicit top-level permissions block in the CI YAML to enforce least privilege (e.g., add a root-level "permissions:" section with at minimum "contents: read") so all jobs (lint, typecheck, build, test) inherit limited token access; update the .github/workflows/ci.yml to include this permissions block at the top of the file.
🧹 Nitpick comments (6)
vitest.config.ts (1)
2-8: Consider inheriting the alias fromvite.config.tsto avoid duplication.The
@alias is defined identically in bothvite.config.tsandvitest.config.ts. Vitest automatically merges config fromvite.config.tswhen no conflictingresolveis present, or you can usemergeConfig. This is a common pattern though, so no action required if you prefer the explicitness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vitest.config.ts` around lines 2 - 8, The vitest config redundantly redefines the same resolve.alias ('@') already present in your Vite config; remove the resolve block from the object passed to defineConfig in vitest.config.ts so Vitest inherits the alias from vite.config.ts, or alternatively import and call mergeConfig(viteConfig, defineConfig(...)) and keep only test-specific settings — target the defineConfig call and the resolve.alias entry (the '@' alias) or use mergeConfig to combine configs instead of duplicating the alias.tsconfig.json (1)
24-24: Typecheck coverage excludes the new build scripts.Line 24 restricts project checking to
src, soscripts/generate-resume-*.tscan bypasstscvalidation. Consider a dedicatedtsconfig.scripts.json(preferred) or expanding include coverage.Minimal patch option
- "include": ["src"] + "include": ["src", "scripts"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` at line 24, tsconfig currently only includes "src", so TypeScript won't typecheck scripts like scripts/generate-resume-*.ts; either create a new tsconfig.scripts.json that extends the base tsconfig and sets "include": ["src", "scripts/**/*.ts"] (preferred) or update the existing "include" to cover "scripts" as well. Add the new tsconfig.scripts.json to CI/npm scripts (or run tsc -p tsconfig.scripts.json) so the generate-resume scripts are validated, and ensure any project references or compilerOptions from the base tsconfig are preserved via "extends".src/components/SiteFooter.tsx (1)
60-60: Redundantgap-2on Button —buttonVariantsbase already includes it.
className="w-fit gap-2"passesgap-2to a Button whosebuttonVariantsbase already specifiesgap-2.tailwind-mergewill deduplicate it, but it adds noise.♻️ Proposed cleanup
- <Button variant="outline" size="sm" className="w-fit gap-2" asChild> + <Button variant="outline" size="sm" className="w-fit" asChild>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SiteFooter.tsx` at line 60, Remove the redundant "gap-2" from the Button's className to avoid passing a duplicate utility already defined in the buttonVariants base; update the Button invocation that currently has className="w-fit gap-2" to only include the differing utility (e.g., "w-fit") so the Button component relies on buttonVariants for spacing.src/components/ui/tabs.tsx (1)
15-16:data-orientationis redundant — Radix sets it automatically.
TabsPrimitive.Rootalready setsdata-orientationon the DOM element based on its ownorientationprop. Manually settingdata-orientation={orientation}on Line 15 is redundant, though harmless.♻️ Proposed cleanup
<TabsPrimitive.Root data-slot="tabs" - data-orientation={orientation} orientation={orientation} className={cn('group/tabs flex gap-2 data-[orientation=horizontal]:flex-col', className)} {...props} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/tabs.tsx` around lines 15 - 16, The manual data-orientation={orientation} prop is redundant because TabsPrimitive.Root already sets data-orientation from its orientation prop; remove the explicit data-orientation={orientation} attribute where TabsPrimitive.Root is rendered (look for the Tabs component / TabsPrimitive.Root usage) so the component only passes orientation={orientation} and relies on Radix to set the data attribute automatically.src/components/ui/scroll-area.tsx (1)
23-23:ScrollAreaalways renders a vertical-onlyScrollBar— horizontal consumers must compose manually.The hardcoded
<ScrollBar />(defaulting toorientation="vertical") means<ScrollArea>cannot display a horizontal scrollbar. This matches standard shadcn conventions but is worth documenting: callers needing horizontal scroll must use<ScrollBar orientation="horizontal" />directly inside<ScrollAreaPrimitive.Root>rather than via this wrapper.If horizontal-scroll use cases are expected (e.g., the career timeline), consider accepting an
orientationprop or a render-prop for the scrollbar slot.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/scroll-area.tsx` at line 23, ScrollArea currently always renders a vertical ScrollBar because the wrapper includes a hardcoded <ScrollBar />; update the ScrollArea component to accept an optional orientation prop (e.g., orientation?: "vertical" | "horizontal" defaulting to "vertical") and pass that value into the rendered <ScrollBar orientation={orientation} /> so callers can request a horizontal scrollbar, or alternatively add a scrollbarSlot prop/render-prop to allow custom scrollbar composition; update the ScrollArea component signature and its internal rendering (references: ScrollArea component, ScrollBar, ScrollAreaPrimitive.Root) and ensure the default behavior remains vertical when the prop is omitted.src/index.css (1)
5-7: Stylelint@custom-variant/@themeerrors are false positives — consider suppressing them.The
scss/at-rule-no-unknownrule fires on Tailwind v4's@custom-variantand@theme inlinedirectives, which are valid CSS-first Tailwind v4 configuration at-rules. Suppressing them at the rule level prevents future noise:🔧 Proposed Stylelint config fix
In your Stylelint config (e.g.,
.stylelintrc), extend thecustomSyntaxor add the at-rules to the ignore list:"rules": { + "scss/at-rule-no-unknown": [true, { + "ignoreAtRules": ["theme", "custom-variant", "source", "utility"] + }] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.css` around lines 5 - 7, Stylelint is flagging Tailwind v4 at-rules (`@custom-variant` and `@theme` inline) as unknown via the scss/at-rule-no-unknown rule; update your Stylelint config to either set customSyntax to a PostCSS/SCSS parser and/or add these at-rules to the ignore list so they are treated as valid—e.g., modify the scss/at-rule-no-unknown rule configuration to include ignoreAtRules entries for "custom-variant" and "theme" (or set customSyntax to "postcss-scss") so occurrences like `@custom-variant` and `@theme` inline in src/index.css are no longer reported as errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 56-65: The "Upload release assets" step currently calls gh release
upload with the files and will fail if assets already exist; update the run
command that invokes gh release upload (the command referenced by TAG_NAME and
the "site-${TAG_NAME}.tar.gz" and "dist/Jon_Bogaty_Resume.*" assets) to include
the --clobber flag so uploads are idempotent on workflow re-runs.
In `@docs/ENTERPRISE.md`:
- Around line 81-83: The checklist item "Create shared branding assets" is
marked pending in the block containing "Create shared branding assets (moved to
dedicated repo)" but elsewhere (the later block around "shared assets were
already applied") it is shown as applied; update the document so both
occurrences of the shared-assets status are consistent — either mark "Create
shared branding assets" as done in the first checklist or change the later
statement that shared assets were applied to reflect that they remain pending.
Locate the phrases "Create shared branding assets (moved to dedicated repo)" and
the later sentence referencing "shared assets were already applied" and make the
statuses match.
In `@package.json`:
- Around line 11-15: The prebuild hook currently runs generate:resume (which
runs generate:resume:pdf and thus Playwright) and can break pnpm build in
environments without Playwright browsers; either remove generate:resume from the
"prebuild" script so prebuild only runs safe tasks (keep "generate:resume" as an
explicit release-only command), or modify the PDF generation path by changing
scripts/generate-resume-pdf.ts to detect the Playwright browser binary (e.g.,
try require('playwright') or check for the browser
executable/PLAYWRIGHT_BROWSERS_PATH) and exit gracefully with code 0 and a log
message if browsers are not installed, ensuring generate:resume:pdf soft-fails
instead of failing the whole build.
In `@src/components/CareerTimeline.tsx`:
- Around line 396-399: The progress-bar width calculation can divide by zero
when entries is empty; update the style logic in CareerTimeline (the div using
activeIndex and entries) to guard against entries.length === 0 by returning a
safe width (e.g., "0%") or using a fallback denominator (e.g., entries.length ||
1) before computing ((activeIndex + 1) / entries.length) * 100, so the rendered
width is valid when the timeline is empty.
- Around line 208-214: The getCardWidth callback hardcodes a 24px gap which
breaks alignment on different screen sizes; update getCardWidth to compute the
actual inter-card gap from the DOM instead of using 24: get the container
element via scrollRef.current, use getComputedStyle on that element and read
either columnGap/rowGap or gap (falling back as needed), parse the value to a
number (px) and add it to firstCard.offsetWidth when returning the width so the
scroll/index calculations match the CSS gap (referencing getCardWidth, scrollRef
and firstCard).
In `@src/components/ProjectsPanel.tsx`:
- Around line 89-101: The CSS variable --project-accent is only being set on the
Badge, so the Card's hover rule hover:border-[var(--project-accent)]/30 in the
ProjectCard component has no value; move or duplicate the inline style that
defines '--project-accent': project.accent from the Badge to the Card container
(the Card element returned by ProjectCard) so the variable is available to the
card and its children, preserving the Badge's current style as needed.
In `@tests/e2e/resume.spec.ts`:
- Around line 36-43: The test "timeline navigation dots are interactive"
currently only verifies visibility; update it so clicking nextButton actually
asserts a state change: after await nextButton.click() assert that secondDot
(the locator named secondDot) reflects an active state (e.g., has the expected
aria attribute like aria-pressed/aria-current or a CSS class such as "active")
and that the previous dot is no longer active or that the active card/content
changed accordingly; use the secondDot locator and the original nextButton
locator to perform these assertions so the test fails on regressions in
navigation behavior.
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 12-108: The workflow currently uses default GITHUB_TOKEN
permissions; add an explicit top-level permissions block in the CI YAML to
enforce least privilege (e.g., add a root-level "permissions:" section with at
minimum "contents: read") so all jobs (lint, typecheck, build, test) inherit
limited token access; update the .github/workflows/ci.yml to include this
permissions block at the top of the file.
---
Nitpick comments:
In `@src/components/SiteFooter.tsx`:
- Line 60: Remove the redundant "gap-2" from the Button's className to avoid
passing a duplicate utility already defined in the buttonVariants base; update
the Button invocation that currently has className="w-fit gap-2" to only include
the differing utility (e.g., "w-fit") so the Button component relies on
buttonVariants for spacing.
In `@src/components/ui/scroll-area.tsx`:
- Line 23: ScrollArea currently always renders a vertical ScrollBar because the
wrapper includes a hardcoded <ScrollBar />; update the ScrollArea component to
accept an optional orientation prop (e.g., orientation?: "vertical" |
"horizontal" defaulting to "vertical") and pass that value into the rendered
<ScrollBar orientation={orientation} /> so callers can request a horizontal
scrollbar, or alternatively add a scrollbarSlot prop/render-prop to allow custom
scrollbar composition; update the ScrollArea component signature and its
internal rendering (references: ScrollArea component, ScrollBar,
ScrollAreaPrimitive.Root) and ensure the default behavior remains vertical when
the prop is omitted.
In `@src/components/ui/tabs.tsx`:
- Around line 15-16: The manual data-orientation={orientation} prop is redundant
because TabsPrimitive.Root already sets data-orientation from its orientation
prop; remove the explicit data-orientation={orientation} attribute where
TabsPrimitive.Root is rendered (look for the Tabs component / TabsPrimitive.Root
usage) so the component only passes orientation={orientation} and relies on
Radix to set the data attribute automatically.
In `@src/index.css`:
- Around line 5-7: Stylelint is flagging Tailwind v4 at-rules (`@custom-variant`
and `@theme` inline) as unknown via the scss/at-rule-no-unknown rule; update your
Stylelint config to either set customSyntax to a PostCSS/SCSS parser and/or add
these at-rules to the ignore list so they are treated as valid—e.g., modify the
scss/at-rule-no-unknown rule configuration to include ignoreAtRules entries for
"custom-variant" and "theme" (or set customSyntax to "postcss-scss") so
occurrences like `@custom-variant` and `@theme` inline in src/index.css are no
longer reported as errors.
In `@tsconfig.json`:
- Line 24: tsconfig currently only includes "src", so TypeScript won't typecheck
scripts like scripts/generate-resume-*.ts; either create a new
tsconfig.scripts.json that extends the base tsconfig and sets "include": ["src",
"scripts/**/*.ts"] (preferred) or update the existing "include" to cover
"scripts" as well. Add the new tsconfig.scripts.json to CI/npm scripts (or run
tsc -p tsconfig.scripts.json) so the generate-resume scripts are validated, and
ensure any project references or compilerOptions from the base tsconfig are
preserved via "extends".
In `@vitest.config.ts`:
- Around line 2-8: The vitest config redundantly redefines the same
resolve.alias ('@') already present in your Vite config; remove the resolve
block from the object passed to defineConfig in vitest.config.ts so Vitest
inherits the alias from vite.config.ts, or alternatively import and call
mergeConfig(viteConfig, defineConfig(...)) and keep only test-specific settings
— target the defineConfig call and the resolve.alias entry (the '@' alias) or
use mergeConfig to combine configs instead of duplicating the alias.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
assets/images/telegram-qr.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/Jon_Bogaty_Resume_2025.docxis excluded by!**/*.docxpublic/Jon_Bogaty_Resume_2025.pdfis excluded by!**/*.pdfpublic/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (84)
.github/workflows/automerge.yml.github/workflows/cd.yml.github/workflows/ci.yml.github/workflows/deploy.yml.github/workflows/docs.yml.github/workflows/release.yml.gitignoreabout.htmlassets/css/style.cssastro.config.mjsbiome.jsoncomponents.jsoncontent/about.mdcontent/ecosystem.ymlcontent/resume.mdcontent/vision.mddocs/ENTERPRISE.mdecosystem.htmlenterprise-branding/README.mdenterprise-branding/org-themes/agentic.cssenterprise-branding/org-themes/arcade.cssenterprise-branding/org-themes/extended-data.cssenterprise-branding/org-themes/strata.cssenterprise-branding/starlight-base.cssenterprise-branding/starlight-enterprise-links.jsonenterprise-branding/templates/astro.config.template.mjsenterprise-branding/templates/custom.css.templateindex.htmlpackage.jsonplaywright.config.tsresume.htmlscripts/generate-resume-docx.tsscripts/generate-resume-pdf.tssrc/App.test.tsxsrc/App.tsxsrc/components/CareerTimeline.tsxsrc/components/Footer.astrosrc/components/Nav.astrosrc/components/ProjectsPanel.tsxsrc/components/SiteFooter.tsxsrc/components/resume/Competencies.astrosrc/components/resume/DownloadBar.astrosrc/components/resume/EarlierCareer.astrosrc/components/resume/Education.astrosrc/components/resume/Experience.astrosrc/components/resume/Header.astrosrc/components/resume/Innovation.astrosrc/components/resume/Projects.astrosrc/components/resume/Skills.astrosrc/components/resume/Summary.astrosrc/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/card.tsxsrc/components/ui/scroll-area.tsxsrc/components/ui/separator.tsxsrc/components/ui/tabs.tsxsrc/data/ecosystem.test.tssrc/data/ecosystem.tssrc/env.d.tssrc/hooks/useDebounce.tssrc/index.csssrc/layouts/BaseLayout.astrosrc/layouts/PrintLayout.astrosrc/lib/docx-builder.tssrc/lib/utils.tssrc/main.tsxsrc/pages/Jon_Bogaty_Resume.docx.tssrc/pages/about.astrosrc/pages/ecosystem.astrosrc/pages/index.astrosrc/pages/resume-print.astrosrc/pages/resume.astrosrc/styles/global.csssrc/styles/print.csssrc/test/setup.tssrc/theme.test.tssrc/theme.tstemplates/resume-pdf.htmltests/e2e/navigation.spec.tstests/e2e/resume.spec.tstsconfig.jsontsconfig.node.jsonvite.config.tsvitest.config.ts
💤 Files with no reviewable changes (52)
- src/components/Footer.astro
- src/hooks/useDebounce.ts
- src/components/resume/Summary.astro
- src/data/ecosystem.test.ts
- content/resume.md
- src/pages/Jon_Bogaty_Resume.docx.ts
- content/vision.md
- src/components/resume/DownloadBar.astro
- src/components/Nav.astro
- src/components/resume/Competencies.astro
- content/about.md
- src/components/resume/Header.astro
- src/components/resume/Experience.astro
- src/components/resume/Skills.astro
- content/ecosystem.yml
- src/pages/resume-print.astro
- src/components/resume/Projects.astro
- .github/workflows/docs.yml
- src/theme.ts
- enterprise-branding/org-themes/agentic.css
- src/data/ecosystem.ts
- tsconfig.node.json
- ecosystem.html
- .github/workflows/deploy.yml
- src/pages/ecosystem.astro
- src/styles/print.css
- enterprise-branding/org-themes/extended-data.css
- enterprise-branding/README.md
- enterprise-branding/org-themes/strata.css
- src/lib/docx-builder.ts
- src/styles/global.css
- about.html
- enterprise-branding/starlight-base.css
- src/components/resume/Education.astro
- assets/css/style.css
- enterprise-branding/org-themes/arcade.css
- src/components/resume/EarlierCareer.astro
- astro.config.mjs
- src/layouts/PrintLayout.astro
- src/App.test.tsx
- src/pages/index.astro
- templates/resume-pdf.html
- enterprise-branding/templates/astro.config.template.mjs
- src/pages/resume.astro
- src/components/resume/Innovation.astro
- enterprise-branding/starlight-enterprise-links.json
- enterprise-branding/templates/custom.css.template
- src/layouts/BaseLayout.astro
- src/pages/about.astro
- resume.html
- src/theme.test.ts
- src/test/setup.ts
| - name: Upload release assets | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| TAG_NAME: ${{ github.event.release.tag_name }} | ||
| run: | | ||
| cd dist && tar czf "../site-${TAG_NAME}.tar.gz" . && cd .. | ||
| gh release upload "$TAG_NAME" \ | ||
| "site-${TAG_NAME}.tar.gz" \ | ||
| "dist/Jon_Bogaty_Resume.pdf" \ | ||
| "dist/Jon_Bogaty_Resume.docx" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
gh release upload --clobber flag documentation
💡 Result:
gh release upload supports --clobber, documented as:
--clobber— “Overwrite existing assets of the same name” (i.e., replace an already-uploaded release asset that has the same filename). [1][2]
Sources: GitHub CLI manual / reference for gh release upload [1] and the gh-release-upload man page mirroring the same option text [2].
🏁 Script executed:
# Check if the workflow file exists and examine full context
cat .github/workflows/release.ymlRepository: jbcom/jbcom.github.io
Length of output: 2150
Add --clobber flag to make release-asset upload idempotent for workflow re-runs.
gh release upload fails when assets already exist, preventing recovery from transient failures. If the workflow re-runs, the upload step fails consistently because the assets were already uploaded in the previous attempt.
Fix
- gh release upload "$TAG_NAME" \
+ gh release upload "$TAG_NAME" --clobber \
"site-${TAG_NAME}.tar.gz" \
"dist/Jon_Bogaty_Resume.pdf" \
"dist/Jon_Bogaty_Resume.docx"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 56 - 65, The "Upload release
assets" step currently calls gh release upload with the files and will fail if
assets already exist; update the run command that invokes gh release upload (the
command referenced by TAG_NAME and the "site-${TAG_NAME}.tar.gz" and
"dist/Jon_Bogaty_Resume.*" assets) to include the --clobber flag so uploads are
idempotent on workflow re-runs.
| - [ ] Create shared branding assets (moved to dedicated repo). | ||
| - [ ] Create division-specific theme overrides. | ||
| - [ ] Create interconnection metadata JSON. |
There was a problem hiding this comment.
Shared-assets status is now inconsistent within the same plan.
Lines 81-83 mark shared assets as pending, but Lines 86-88 still indicate shared assets were already applied. Please align these statuses so the triage state is unambiguous.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/ENTERPRISE.md` around lines 81 - 83, The checklist item "Create shared
branding assets" is marked pending in the block containing "Create shared
branding assets (moved to dedicated repo)" but elsewhere (the later block around
"shared assets were already applied") it is shown as applied; update the
document so both occurrences of the shared-assets status are consistent — either
mark "Create shared branding assets" as done in the first checklist or change
the later statement that shared assets were applied to reflect that they remain
pending. Locate the phrases "Create shared branding assets (moved to dedicated
repo)" and the later sentence referencing "shared assets were already applied"
and make the statuses match.
| "prebuild": "pnpm generate:resume", | ||
| "preview": "vite preview", | ||
| "generate:resume": "pnpm generate:resume:docx && pnpm generate:resume:pdf", | ||
| "generate:resume:docx": "tsx scripts/generate-resume-docx.ts", | ||
| "generate:resume:pdf": "tsx scripts/generate-resume-pdf.ts", |
There was a problem hiding this comment.
prebuild runs Playwright-dependent PDF generation on every pnpm build.
generate:resume:pdf invokes Playwright, which requires browser binaries installed via playwright install. Any environment that hasn't run that step (fresh CI runner, clean checkout, local dev without browsers) will hard-fail pnpm build entirely before even touching the app code.
Consider guarding resume generation so it only runs in release contexts, or add an early-exit if the Playwright executable is missing:
🛡️ Proposed mitigation options
Option A — decouple prebuild from resume generation:
- "prebuild": "pnpm generate:resume",
"build": "vite build",Then invoke pnpm generate:resume && pnpm build only in the release workflow.
Option B — soft-fail the PDF step when browsers are absent:
- "generate:resume:pdf": "tsx scripts/generate-resume-pdf.ts",
+ "generate:resume:pdf": "playwright install chromium --with-deps || true && tsx scripts/generate-resume-pdf.ts",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "prebuild": "pnpm generate:resume", | |
| "preview": "vite preview", | |
| "generate:resume": "pnpm generate:resume:docx && pnpm generate:resume:pdf", | |
| "generate:resume:docx": "tsx scripts/generate-resume-docx.ts", | |
| "generate:resume:pdf": "tsx scripts/generate-resume-pdf.ts", | |
| "preview": "vite preview", | |
| "build": "vite build", | |
| "generate:resume": "pnpm generate:resume:docx && pnpm generate:resume:pdf", | |
| "generate:resume:docx": "tsx scripts/generate-resume-docx.ts", | |
| "generate:resume:pdf": "tsx scripts/generate-resume-pdf.ts", |
| "prebuild": "pnpm generate:resume", | |
| "preview": "vite preview", | |
| "generate:resume": "pnpm generate:resume:docx && pnpm generate:resume:pdf", | |
| "generate:resume:docx": "tsx scripts/generate-resume-docx.ts", | |
| "generate:resume:pdf": "tsx scripts/generate-resume-pdf.ts", | |
| "prebuild": "pnpm generate:resume", | |
| "preview": "vite preview", | |
| "generate:resume": "pnpm generate:resume:docx && pnpm generate:resume:pdf", | |
| "generate:resume:docx": "tsx scripts/generate-resume-docx.ts", | |
| "generate:resume:pdf": "playwright install chromium --with-deps || true && tsx scripts/generate-resume-pdf.ts", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 11 - 15, The prebuild hook currently runs
generate:resume (which runs generate:resume:pdf and thus Playwright) and can
break pnpm build in environments without Playwright browsers; either remove
generate:resume from the "prebuild" script so prebuild only runs safe tasks
(keep "generate:resume" as an explicit release-only command), or modify the PDF
generation path by changing scripts/generate-resume-pdf.ts to detect the
Playwright browser binary (e.g., try require('playwright') or check for the
browser executable/PLAYWRIGHT_BROWSERS_PATH) and exit gracefully with code 0 and
a log message if browsers are not installed, ensuring generate:resume:pdf
soft-fails instead of failing the whole build.
| const getCardWidth = useCallback(() => { | ||
| const el = scrollRef.current | ||
| if (!el) return 400 | ||
| const firstCard = el.querySelector(':scope > *') as HTMLElement | null | ||
| if (!firstCard) return 400 | ||
| return firstCard.offsetWidth + 24 // card width + gap | ||
| }, []) |
There was a problem hiding this comment.
Do not hard-code the inter-card gap in scroll/index calculations.
Line 213 assumes 24px, but the track uses gap-4 sm:gap-6 (Line 425). On small screens this misaligns active index and arrow/dot navigation.
💡 Proposed fix
const getCardWidth = useCallback(() => {
const el = scrollRef.current
if (!el) return 400
const firstCard = el.querySelector(':scope > *') as HTMLElement | null
if (!firstCard) return 400
- return firstCard.offsetWidth + 24 // card width + gap
+ const styles = window.getComputedStyle(el)
+ const gap = Number.parseFloat(styles.columnGap || styles.gap || '0') || 0
+ return firstCard.offsetWidth + gap
}, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/CareerTimeline.tsx` around lines 208 - 214, The getCardWidth
callback hardcodes a 24px gap which breaks alignment on different screen sizes;
update getCardWidth to compute the actual inter-card gap from the DOM instead of
using 24: get the container element via scrollRef.current, use getComputedStyle
on that element and read either columnGap/rowGap or gap (falling back as
needed), parse the value to a number (px) and add it to firstCard.offsetWidth
when returning the width so the scroll/index calculations match the CSS gap
(referencing getCardWidth, scrollRef and firstCard).
| <div | ||
| className="h-full bg-gradient-to-r from-primary to-primary/60 rounded-full transition-all duration-300" | ||
| style={{ width: `${((activeIndex + 1) / entries.length) * 100}%` }} | ||
| /> |
There was a problem hiding this comment.
Guard progress width for empty timelines.
Line 398 divides by entries.length; when entries is empty this yields invalid width (Infinity%).
💡 Proposed fix
className="h-full bg-gradient-to-r from-primary to-primary/60 rounded-full transition-all duration-300"
- style={{ width: `${((activeIndex + 1) / entries.length) * 100}%` }}
+ style={{
+ width: `${entries.length > 0 ? ((activeIndex + 1) / entries.length) * 100 : 0}%`,
+ }}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| className="h-full bg-gradient-to-r from-primary to-primary/60 rounded-full transition-all duration-300" | |
| style={{ width: `${((activeIndex + 1) / entries.length) * 100}%` }} | |
| /> | |
| <div | |
| className="h-full bg-gradient-to-r from-primary to-primary/60 rounded-full transition-all duration-300" | |
| style={{ | |
| width: `${entries.length > 0 ? ((activeIndex + 1) / entries.length) * 100 : 0}%`, | |
| }} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/CareerTimeline.tsx` around lines 396 - 399, The progress-bar
width calculation can divide by zero when entries is empty; update the style
logic in CareerTimeline (the div using activeIndex and entries) to guard against
entries.length === 0 by returning a safe width (e.g., "0%") or using a fallback
denominator (e.g., entries.length || 1) before computing ((activeIndex + 1) /
entries.length) * 100, so the rendered width is valid when the timeline is
empty.
| function ProjectCard({ project }: { project: Project }) { | ||
| return ( | ||
| <Card className="border-border hover:border-[var(--project-accent)]/30 transition-all duration-300 group overflow-hidden relative"> | ||
| {/* Top accent */} | ||
| <div className="absolute top-0 left-0 right-0 h-0.5" style={{ background: project.accent }} /> | ||
|
|
||
| <CardHeader> | ||
| <div className="flex items-center justify-between"> | ||
| <Badge | ||
| variant="outline" | ||
| className="border-[var(--project-accent)]/25 text-[var(--project-accent)] text-[0.65rem]" | ||
| style={{ '--project-accent': project.accent } as React.CSSProperties} | ||
| > |
There was a problem hiding this comment.
Define --project-accent on the card container.
Line 91 uses hover:border-[var(--project-accent)]/30, but the variable is only set on the badge (Line 100), so card hover accent is currently ineffective.
💡 Proposed fix
function ProjectCard({ project }: { project: Project }) {
+ const accentVars = { '--project-accent': project.accent } as { '--project-accent': string }
+
return (
- <Card className="border-border hover:border-[var(--project-accent)]/30 transition-all duration-300 group overflow-hidden relative">
+ <Card
+ className="border-border hover:border-[var(--project-accent)]/30 transition-all duration-300 group overflow-hidden relative"
+ style={accentVars}
+ >
{/* Top accent */}
<div className="absolute top-0 left-0 right-0 h-0.5" style={{ background: project.accent }} />
@@
<Badge
variant="outline"
className="border-[var(--project-accent)]/25 text-[var(--project-accent)] text-[0.65rem]"
- style={{ '--project-accent': project.accent } as React.CSSProperties}
+ style={accentVars}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function ProjectCard({ project }: { project: Project }) { | |
| return ( | |
| <Card className="border-border hover:border-[var(--project-accent)]/30 transition-all duration-300 group overflow-hidden relative"> | |
| {/* Top accent */} | |
| <div className="absolute top-0 left-0 right-0 h-0.5" style={{ background: project.accent }} /> | |
| <CardHeader> | |
| <div className="flex items-center justify-between"> | |
| <Badge | |
| variant="outline" | |
| className="border-[var(--project-accent)]/25 text-[var(--project-accent)] text-[0.65rem]" | |
| style={{ '--project-accent': project.accent } as React.CSSProperties} | |
| > | |
| function ProjectCard({ project }: { project: Project }) { | |
| const accentVars = { '--project-accent': project.accent } as { '--project-accent': string } | |
| return ( | |
| <Card | |
| className="border-border hover:border-[var(--project-accent)]/30 transition-all duration-300 group overflow-hidden relative" | |
| style={accentVars} | |
| > | |
| {/* Top accent */} | |
| <div className="absolute top-0 left-0 right-0 h-0.5" style={{ background: project.accent }} /> | |
| <CardHeader> | |
| <div className="flex items-center justify-between"> | |
| <Badge | |
| variant="outline" | |
| className="border-[var(--project-accent)]/25 text-[var(--project-accent)] text-[0.65rem]" | |
| style={accentVars} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ProjectsPanel.tsx` around lines 89 - 101, The CSS variable
--project-accent is only being set on the Badge, so the Card's hover rule
hover:border-[var(--project-accent)]/30 in the ProjectCard component has no
value; move or duplicate the inline style that defines '--project-accent':
project.accent from the Badge to the Card container (the Card element returned
by ProjectCard) so the variable is available to the card and its children,
preserving the Badge's current style as needed.
| test('timeline navigation dots are interactive', async ({ page }) => { | ||
| const nextButton = page.getByRole('button', { name: 'Next' }) | ||
| await expect(nextButton).toBeVisible() | ||
| await nextButton.click() | ||
| // After clicking next, the second card should become active | ||
| const secondDot = page.getByRole('button', { name: /Jump to Flipside Crypto/i }).nth(1) | ||
| await expect(secondDot).toBeVisible() | ||
| }) |
There was a problem hiding this comment.
This interaction test does not actually verify state change.
Line 42 only checks visibility of the second dot, which is already visible before clicking Next; it won’t catch regressions in navigation behavior.
💡 Proposed fix
test('timeline navigation dots are interactive', async ({ page }) => {
const nextButton = page.getByRole('button', { name: 'Next' })
await expect(nextButton).toBeVisible()
+ const dots = page.getByRole('button', { name: /Jump to Flipside Crypto/i })
+ await expect(dots.nth(0)).toHaveClass(/bg-primary\/10/)
await nextButton.click()
- // After clicking next, the second card should become active
- const secondDot = page.getByRole('button', { name: /Jump to Flipside Crypto/i }).nth(1)
- await expect(secondDot).toBeVisible()
+ await expect(dots.nth(1)).toHaveClass(/bg-primary\/10/)
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('timeline navigation dots are interactive', async ({ page }) => { | |
| const nextButton = page.getByRole('button', { name: 'Next' }) | |
| await expect(nextButton).toBeVisible() | |
| await nextButton.click() | |
| // After clicking next, the second card should become active | |
| const secondDot = page.getByRole('button', { name: /Jump to Flipside Crypto/i }).nth(1) | |
| await expect(secondDot).toBeVisible() | |
| }) | |
| test('timeline navigation dots are interactive', async ({ page }) => { | |
| const nextButton = page.getByRole('button', { name: 'Next' }) | |
| await expect(nextButton).toBeVisible() | |
| const dots = page.getByRole('button', { name: /Jump to Flipside Crypto/i }) | |
| await expect(dots.nth(0)).toHaveClass(/bg-primary\/10/) | |
| await nextButton.click() | |
| await expect(dots.nth(1)).toHaveClass(/bg-primary\/10/) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/resume.spec.ts` around lines 36 - 43, The test "timeline navigation
dots are interactive" currently only verifies visibility; update it so clicking
nextButton actually asserts a state change: after await nextButton.click()
assert that secondDot (the locator named secondDot) reflects an active state
(e.g., has the expected aria attribute like aria-pressed/aria-current or a CSS
class such as "active") and that the previous dot is no longer active or that
the active card/content changed accordingly; use the secondDot locator and the
original nextButton locator to perform these assertions so the test fails on
regressions in navigation behavior.
Summary
Complete architecture migration from Astro SSG multi-page site to a single-page Vite + React + shadcn/ui application.
Tech Stack
Breaking Changes
Test plan
pnpm buildproduces clean dist/pnpm lintpasses (biome)pnpm checkpasses (tsc)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor