feat: add social share buttons to footer#149
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbitRelease Notes
WalkthroughThis PR introduces a comprehensive testing and component documentation infrastructure by adding Storybook for interactive UI component development, Playwright-based end-to-end testing via CI, new UI components (CookieConsent, ShareButtons), accessibility enhancements (skip-to-content link), and supporting tooling configuration across ESLint, Vite, and package dependencies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| name: E2E Tests | ||
| runs-on: ubuntu-latest | ||
| needs: [build] | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' | ||
| cache: 'npm' | ||
| - run: npm ci | ||
| - run: npx playwright install --with-deps | ||
| - run: npm run e2e |
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 add an explicit permissions block that grants only the minimal scopes required. For a typical CI pipeline that just checks out code and runs builds/tests, contents: read is sufficient. Since all jobs in this workflow share that same pattern and none push code, create releases, or interact with issues/PRs via the token, we can safely define a single top-level permissions block that applies to all jobs.
The best fix with no functional change is: in .github/workflows/ci.yml, add a root-level permissions: section after the name: (or after on:) that sets contents: read. This documents the intended permissions, ensures they remain restricted if repository defaults change, and addresses the CodeQL warning for all jobs, including the highlighted e2e job. No additional imports, tools, or code changes are needed.
Concretely: edit .github/workflows/ci.yml to insert:
permissions:
contents: readat the top workflow level (aligned with name and on). No other lines need to be modified.
| @@ -1,5 +1,8 @@ | ||
| name: CI | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 3 medium |
| Complexity | 1 medium |
🟢 Metrics 8 complexity . 0 duplication
Metric Results Complexity 8 Duplication 0
TIP This summary will be updated as you push new changes. Give us feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 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/ci.yml:
- Around line 59-71: The e2e job currently exposes a full GITHUB_TOKEN scope;
add an explicit minimal permissions block to the e2e job to limit token scope
(e.g., under the e2e job define permissions: contents: read and any other
minimal scopes your tests actually require). Update the job named "e2e" to
include this permissions map (adjust scopes only if your Playwright/e2e steps
require additional read/write access) so GITHUB_TOKEN is restricted during the
Run steps and npm/playwright/e2e commands.
- Line 62: The CI job currently declares needs: [build] even though the e2e
tests (per playwright.config.ts) run npm run dev against port 5173 and do not
consume build artifacts; either remove needs: [build] so e2e runs in parallel
with build, or (preferred) change the e2e workflow and playwright.config.ts to
run a production preview: have the CI build produce dist/, start the server with
vite preview (serving dist/) and point Playwright to that URL so the tests
validate the production build instead of the dev server.
In `@eslint.config.js`:
- Around line 34-45: Replace the manual browser global list in the eslint
config's globals object with the packaged browser set: import or reference
globals.browser and assign it instead of the explicit properties (the existing
globals: { window: ..., React: ... } block); also update the other
TypeScript/TSX globals block (the **/*.{ts,tsx} globals section) to use
globals.browser as well so both places rely on the canonical browser globals
export.
In `@package.json`:
- Line 29: The package versions for the Playwright packages are inconsistent:
align the versions of "@playwright/test" and "playwright" so they match (e.g.,
set both to the same semver like "^1.59.1" or "^1.40.0") to avoid runner/browser
binary mismatches; update the package.json entries for "@playwright/test" and
"playwright" to the same version, run a quick install to update lockfile, and
verify no other Playwright-related dependencies conflict.
In `@src/App.tsx`:
- Line 75: App mounts <CookieConsent /> but analytics still mounts
unconditionally, so consent (stored under "docugen-cookie-consent" by
CookieConsent) is never checked; update the mounting logic so Analytics only
initializes when consent is granted. Concretely: read the consent state
(localStorage key "docugen-cookie-consent" or expose a getter from
CookieConsent) and gate the Analytics component (or its script injection) behind
that check — either pass a boolean prop from App to <Analytics consent={...}> or
modify Analytics.tsx to read the same storage key before injecting scripts;
ensure the component names involved are CookieConsent, Analytics and the storage
key "docugen-cookie-consent".
- Line 10: Update the import in App.tsx to use the project's absolute-import
convention: replace the relative import of CookieConsent with an absolute one
from the src root (import { CookieConsent } from
'src/components/CookieConsent'), ensuring the symbol CookieConsent and its
module path are updated accordingly.
In `@src/components/Footer.tsx`:
- Line 3: The import in Footer.tsx uses a relative path; update the import of
ShareButtons to use the repo's configured absolute src import (e.g., replace
"import { ShareButtons } from './ShareButtons';" with an absolute path like
"import { ShareButtons } from 'src/components/ShareButtons';") so the Footer
component uses the src/ absolute import convention.
In `@src/components/Hero.tsx`:
- Line 97: The overlay div using raw palette classes ("absolute inset-0
animated-gradient bg-gradient-to-br from-slate-900 via-slate-950 to-teal-950/30
pointer-events-none") should use your design-system semantic tokens instead;
locate the div in the Hero component (the element with className containing
"animated-gradient" and the from-/via-/to- palette classes) and replace the raw
colors with semantic Tailwind tokens defined in your tailwind.config (for
example use tokens like from-[semantic-start] via-[semantic-mid]
to-[semantic-end] or from-primary-XXX/via-surface-YYY/to-overlay-ZZZ), keep
layout classes (absolute inset-0, animated-gradient, pointer-events-none)
unchanged, and if the semantic tokens do not exist add them to tailwind.config
under theme.extend.colors before using.
In `@src/components/ui/Button.stories.tsx`:
- Line 1: The Storybook type import in Button.stories.tsx currently imports Meta
and StoryObj from '@storybook/react' which is inconsistent with the project
convention; update the import to pull Meta and StoryObj from
'@storybook/react-vite' (matching Container.stories.tsx and other stories) so
types align across stories, then run type-check/TSLint to ensure no remaining
import/type errors.
In `@src/components/ui/Container.stories.tsx`:
- Line 1: The story file imports Storybook types from '@storybook/react' which
is inconsistent with other stories; update the import source in
Container.stories.tsx so the named types Meta and StoryObj are imported from
'@storybook/react-vite' (same as Header.stories.ts and Page.stories.ts) to
maintain consistent Storybook setup and avoid bundler/runtime mismatches.
In `@src/components/ui/Input.stories.tsx`:
- Line 66: The story passes aria-invalid as a string which mismatches InputProps
(boolean); change the prop on the Input component in Input.stories.tsx from
aria-invalid="true" to a boolean value (aria-invalid={true}) so it matches the
InputProps type and avoids type errors; verify the Input component and
InputProps interface expect a boolean and update any other story instances using
aria-invalid similarly.
In `@src/stories/Button.tsx`:
- Line 1: Remove the unused default React import at the top of Button.tsx (the
"import React from 'react';" line); because the project uses the modern JSX
transform you can delete this import and instead ensure any React-specific
symbols in this file (e.g., React.FC, React.ReactNode) are either removed or
replaced with named imports like FC/ReactNode from 'react' or plain
function/component definitions (mirror the change made in Header.tsx).
In `@src/stories/Configure.mdx`:
- Around line 50-54: All external anchor tags that open in a new tab (those with
target="_blank" such as the <a className="sb-action-link" ...> links and other
anchors in Configure.mdx) must include rel="noopener noreferrer"; update every
anchor in Configure.mdx that uses target="_blank" (e.g., the "Learn more" link
that uses RightArrow and any similar anchors) to add rel="noopener noreferrer"
to prevent the opened page from accessing window.opener.
- Around line 22-29: The inline style object in Configure.mdx contains an
invalid property key 'path fill' which browsers ignore; remove the "'path fill':
'currentColor'" entry from the style object (leaving fill: 'currentColor') so
path elements inherit the color correctly; locate the style object where the JSX
sets style={{ marginLeft: '4px', display: 'inline-block', shapeRendering:
'inherit', verticalAlign: 'middle', fill: 'currentColor', 'path fill':
'currentColor' }} and delete the 'path fill' line.
In `@src/stories/Header.tsx`:
- Line 1: Remove the unused top-level import statement "import React from
'react';" in Header.tsx since the file's Header component does not reference
React APIs directly; keep the component declaration (Header) and any existing
exports intact and run a quick lint/format to ensure no unused-import errors
remain.
In `@src/stories/Page.tsx`:
- Around line 6-8: Replace the object type alias "type User = { name: string }"
with an equivalent interface declaration to follow the coding guideline; locate
the "User" symbol in src/stories/Page.tsx and change it to "interface User {
name: string }" wherever it's defined/used so references remain compatible.
In `@vite.config.ts`:
- Around line 8-9: The ternary initializing the dirname constant uses a negated
typeof check; update the expression to test for typeof __dirname === 'undefined'
and swap the true/false branches so the logic is not negated. Locate the dirname
declaration (symbol: dirname) that references __dirname and
fileURLToPath(import.meta.url) and replace the ternary so the branch producing
path.dirname(fileURLToPath(import.meta.url)) runs when __dirname is undefined
and __dirname is used otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 437e86b5-b3f9-4cfe-9034-51b9a0e2535b
⛔ Files ignored due to path filters (16)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/stories/assets/accessibility.pngis excluded by!**/*.pngsrc/stories/assets/accessibility.svgis excluded by!**/*.svgsrc/stories/assets/addon-library.pngis excluded by!**/*.pngsrc/stories/assets/assets.pngis excluded by!**/*.pngsrc/stories/assets/context.pngis excluded by!**/*.pngsrc/stories/assets/discord.svgis excluded by!**/*.svgsrc/stories/assets/docs.pngis excluded by!**/*.pngsrc/stories/assets/figma-plugin.pngis excluded by!**/*.pngsrc/stories/assets/github.svgis excluded by!**/*.svgsrc/stories/assets/share.pngis excluded by!**/*.pngsrc/stories/assets/styling.pngis excluded by!**/*.pngsrc/stories/assets/testing.pngis excluded by!**/*.pngsrc/stories/assets/theming.pngis excluded by!**/*.pngsrc/stories/assets/tutorials.svgis excluded by!**/*.svgsrc/stories/assets/youtube.svgis excluded by!**/*.svg
📒 Files selected for processing (27)
.github/workflows/ci.yml.gitignore.storybook/main.ts.storybook/preview.tseslint.config.jsindex.htmlpackage.jsonsrc/App.tsxsrc/components/Footer.tsxsrc/components/Hero.tsxsrc/components/ui/Button.stories.tsxsrc/components/ui/Container.stories.tsxsrc/components/ui/Input.stories.tsxsrc/index.csssrc/stories/Button.stories.tssrc/stories/Button.tsxsrc/stories/Configure.mdxsrc/stories/Header.stories.tssrc/stories/Header.tsxsrc/stories/Page.stories.tssrc/stories/Page.tsxsrc/stories/assets/avif-test-image.avifsrc/stories/button.csssrc/stories/header.csssrc/stories/page.cssvite.config.tsvitest.shims.d.ts
📜 Review details
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: Codacy Static Code Analysis
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Use absolute imports fromsrc/directory (configured intsconfig.json)
Organize imports in this order: React → external dependencies → internal components/utils
Use camelCase for variable and function names (e.g.,handleSubmit,isLoading)
Use SCREAMING_SNAKE_CASE for constants (e.g.,SITE_CONFIG)
Handle runtime errors properly — avoid emptycatchblocks; always log the error or re-throw to preserve stack traces
Files:
src/components/Footer.tsxsrc/App.tsxsrc/components/Hero.tsxsrc/stories/Page.tsxsrc/components/ui/Container.stories.tsxsrc/stories/Page.stories.tssrc/components/ui/Button.stories.tsxsrc/components/ui/Input.stories.tsxsrc/stories/Header.stories.tssrc/stories/Button.tsxsrc/stories/Header.tsxsrc/stories/Button.stories.ts
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
src/components/**/*.tsx: Use named exports only for components; no default exports
Use PascalCase for component files and component names (e.g.,HeroSection,PricingCard)
Use meaningful CSS class names, avoid abbreviations
Use design system colors fromtailwind.config.js(e.g.,text-dark-100,bg-teal-600)
Dark mode first: The app defaults to dark mode. Usedark:prefix to activate styles when dark mode is active.
Avoid arbitrary Tailwind values ([...]); extend theme instead
Use semantic Tailwind color names:text-dark-300for secondary text,text-teal-400for accents
Use consistent spacing scale:4,6,8,12,16
Use responsive Tailwind prefixes:sm:,md:,lg:for responsive breakpoints
Use Framer Motion for entrance animations only (fade-in, slide-up); keep duration at0.5sfor most,0.6sfor complex animations
When using Framer Motion, stagger children withdelay: index * 0.1
Use inline form validation or simple state for form validation; do not use external validation libraries
Display user-friendly messages for network errors
Place all text copy and site configuration insrc/data/content.ts; never hardcode text in components
Use mobile-first responsive design approach
UseReact.memo()for expensive components
Optimize images with WebP/AVIF formats
Files:
src/components/Footer.tsxsrc/components/Hero.tsxsrc/components/ui/Container.stories.tsxsrc/components/ui/Button.stories.tsxsrc/components/ui/Input.stories.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Noanytypes - use explicit types orunknownwith type guards
Useinterfacefor object types,typefor unions/primitives
Use descriptive generic names (T,K,V) or descriptive names (Item,Key) for generics
Files:
src/components/Footer.tsxsrc/App.tsxsrc/components/Hero.tsxsrc/stories/Page.tsxsrc/components/ui/Container.stories.tsxsrc/stories/Page.stories.tssrc/components/ui/Button.stories.tsxsrc/components/ui/Input.stories.tsxsrc/stories/Header.stories.tssrc/stories/Button.tsxsrc/stories/Header.tsxsrc/stories/Button.stories.ts
🧠 Learnings (12)
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/**/*.tsx : Dark mode first: The app defaults to dark mode. Use `dark:` prefix to activate styles when dark mode is active.
Applied to files:
index.html
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/**/*.tsx : Place all text copy and site configuration in `src/data/content.ts`; never hardcode text in components
Applied to files:
src/App.tsx
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/**/*.{ts,tsx,js,jsx} : Organize imports in this order: React → external dependencies → internal components/utils
Applied to files:
src/App.tsxeslint.config.js
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/sections/**/*.tsx : Lazy load sections: Features, Pricing, Testimonials, Preview, Newsletter using `React.lazy()`
Applied to files:
src/App.tsx
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Run `npm run test:run` before committing
Applied to files:
.github/workflows/ci.ymlpackage.json
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/**/*.tsx : Use Framer Motion for entrance animations only (fade-in, slide-up); keep duration at `0.5s` for most, `0.6s` for complex animations
Applied to files:
src/index.css
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/sections/**/*.tsx : No complex gesture animations on landing pages
Applied to files:
src/index.csssrc/components/Hero.tsx
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/**/*.tsx : Use semantic Tailwind color names: `text-dark-300` for secondary text, `text-teal-400` for accents
Applied to files:
src/components/Hero.tsx
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/**/*.tsx : Use design system colors from `tailwind.config.js` (e.g., `text-dark-100`, `bg-teal-600`)
Applied to files:
src/components/Hero.tsx
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/**/*.tsx : Avoid arbitrary Tailwind values (`[...]`); extend theme instead
Applied to files:
src/components/Hero.tsx
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/sections/**/*.tsx : Use Framer Motion viewport options: `viewport={{ once: true }}` for landing pages
Applied to files:
src/stories/Page.tsx
📚 Learning: 2026-02-17T16:01:51.517Z
Learnt from: CR
Repo: shazzar00ni/docugen PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T16:01:51.517Z
Learning: Applies to src/components/**/*.test.tsx : Place test files next to components with `.test.tsx` extension (e.g., `src/components/ui/Button.test.tsx`)
Applied to files:
src/components/ui/Button.stories.tsxsrc/stories/Button.stories.ts
🪛 GitHub Check: CodeQL
.github/workflows/ci.yml
[warning] 60-71: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
🪛 GitHub Check: SonarCloud Code Analysis
vite.config.ts
[warning] 9-9: Unexpected negated condition.
src/stories/Page.tsx
[warning] 54-54: Ambiguous spacing after previous element a
🪛 LanguageTool
src/stories/Configure.mdx
[uncategorized] ~3-~3: The official name of this software platform is spelled with a capital “H”.
Context: ..."@storybook/addon-docs/blocks"; import Github from "./assets/github.svg"; import Disc...
(GITHUB)
[uncategorized] ~3-~3: The official name of this software platform is spelled with a capital “H”.
Context: .../blocks"; import Github from "./assets/github.svg"; import Discord from "./assets/dis...
(GITHUB)
[uncategorized] ~5-~5: The official name of this popular video platform is spelled with a capital “T”.
Context: ...ord from "./assets/discord.svg"; import Youtube from "./assets/youtube.svg"; import Tut...
(YOUTUBE)
[uncategorized] ~5-~5: The official name of this popular video platform is spelled with a capital “T”.
Context: ...ord.svg"; import Youtube from "./assets/youtube.svg"; import Tutorials from "./assets/t...
(YOUTUBE)
🔇 Additional comments (24)
src/index.css (1)
28-50: Reduced-motion fallback is correctly implemented.The animation utility is cleanly defined, and the
prefers-reduced-motionoverride is a solid accessibility safeguard.src/components/Footer.tsx (1)
63-63:ShareButtonsplacement in footer actions looks good.This insertion is clean and matches the stated feature objective.
index.html (1)
32-37: Skip-link implementation is solid.Focusable skip navigation is correctly wired to the main landmark target and improves keyboard accessibility.
src/App.tsx (1)
54-54: Main landmark target is correctly added.
id="main-content"witharia-labelaligns with the skip-link target and improves navigability..gitignore (1)
3-5: LGTM!Appropriate additions for ignoring Storybook build artifacts and logs.
.storybook/preview.ts (1)
12-17: Consider enabling a11y enforcement in CI eventually.Setting
test: 'todo'is fine for initial setup. Once accessibility issues are addressed, change to'error'to fail CI on violations and prevent regressions.vitest.shims.d.ts (1)
1-1: LGTM!Correct use of triple-slash directive to include Vitest browser-playwright type definitions.
src/stories/header.css (1)
1-32: LGTM!Well-structured CSS with appropriately scoped selectors for the Storybook header component.
src/stories/Page.tsx (1)
10-73: LGTM!Component logic is clean. External links correctly use
rel="noopener noreferrer"withtarget="_blank". The{' '}patterns for JSX text spacing are intentional and correct—the static analysis warning is a false positive..storybook/main.ts (1)
1-17: LGTM!Well-structured Storybook configuration with appropriate addons for accessibility testing, documentation, and Chromatic integration.
src/stories/Header.stories.ts (1)
1-34: LGTM!Story file follows Storybook CSF conventions correctly. The default export for meta is required by Storybook, and the
satisfies Meta<typeof Header>pattern provides good type safety. Mock callbacks viafn()are properly configured for interaction testing.src/stories/button.css (1)
1-30: LGTM!Standard Storybook example styling. These are isolated demo styles for the example Button component, separate from the production design system in
src/components/.vite.config.ts (1)
22-55: LGTM!The multi-project Vitest configuration is well-structured. The first project preserves existing jsdom unit test behavior with proper setup files, while the second integrates Storybook testing via Playwright. The
extends: truecorrectly inherits coverage settings.src/components/ui/Container.stories.tsx (1)
16-42: LGTM!Stories demonstrate the Container component's API effectively. The
Default,WithCustomClass, andGridExamplestories cover the primary use cases and align with the component's interface shown in the relevant code snippet.src/stories/Page.stories.ts (1)
1-33: LGTM!The interaction test correctly validates the login/logout toggle behavior. The
playfunction properly scopes queries tocanvasElementviawithin(), and the assertions align with the Page component's state management shown in the context snippets.src/stories/page.css (1)
1-68: LGTM!Standard Storybook example page styling. These isolated demo styles are appropriate for the example Page component.
src/components/ui/Button.stories.tsx (1)
4-86: LGTM!Comprehensive story coverage for the Button component. The
argTypesconfiguration provides useful controls, and the composite stories (AllVariants,AllSizes) effectively showcase the component's variations.package.json (1)
30-32: The current version configuration is correct.@storybook/testis a utility package designed to work across Storybook major versions and does not require version alignment with@storybook/builder-viteor@storybook/react. Version 8.6.15 is compatible with Storybook 10.x, and no 10.x release of@storybook/testexists. No changes needed.> Likely an incorrect or invalid review comment.src/stories/Configure.mdx (2)
3-5: Static analysis false positive: import names are valid identifiers.The LanguageTool hints about "Github" and "Youtube" capitalization refer to the import variable names, which must match the filenames. These are local identifiers, not user-facing text, so the capitalization is acceptable.
1-388: Standard Storybook onboarding MDX content.This appears to be boilerplate documentation from Storybook's scaffolding. The content provides useful onboarding guidance for developers new to Storybook.
src/components/ui/Input.stories.tsx (1)
1-69: Storybook stories are well-structured with good coverage of input states.The stories cover default, email, disabled, pre-filled value, error state, and a combined view. ARIA attributes are correctly typed in
ErrorState(lines 55-56).src/stories/Header.tsx (1)
17-56: Component structure is clean with proper conditional rendering.The Header correctly handles both authenticated and unauthenticated states with appropriate UI elements.
src/stories/Button.tsx (1)
5-37: Well-documented component with clear TypeScript types.The JSDoc comments on props are helpful for Storybook's autodocs feature, and the implementation is straightforward.
src/stories/Button.stories.ts (1)
1-54: Standard Storybook CSF3 implementation.The story configuration is correct with proper typing, autodocs enabled, and action spies wired via
fn(). The four variants provide good coverage of the button states.
| e2e: | ||
| name: E2E Tests | ||
| runs-on: ubuntu-latest | ||
| needs: [build] | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' | ||
| cache: 'npm' | ||
| - run: npm ci | ||
| - run: npx playwright install --with-deps | ||
| - run: npm run e2e |
There was a problem hiding this comment.
Add explicit permissions to limit GITHUB_TOKEN scope.
The CodeQL analysis correctly flags that this job lacks a permissions block. Adding minimal permissions improves security posture.
🛡️ Proposed fix
e2e:
name: E2E Tests
runs-on: ubuntu-latest
needs: [build]
+ permissions:
+ contents: read
steps:🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 60-71: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 59 - 71, The e2e job currently exposes
a full GITHUB_TOKEN scope; add an explicit minimal permissions block to the e2e
job to limit token scope (e.g., under the e2e job define permissions: contents:
read and any other minimal scopes your tests actually require). Update the job
named "e2e" to include this permissions map (adjust scopes only if your
Playwright/e2e steps require additional read/write access) so GITHUB_TOKEN is
restricted during the Run steps and npm/playwright/e2e commands.
| e2e: | ||
| name: E2E Tests | ||
| runs-on: ubuntu-latest | ||
| needs: [build] |
There was a problem hiding this comment.
needs: [build] is misleading—e2e tests don't use build artifacts.
Per playwright.config.ts, the e2e tests run npm run dev (port 5173), starting their own development server. The build job's artifacts are not consumed. Consider either:
- Remove
needs: [build]and run e2e in parallel with build, or - Update playwright config to use
vite previewagainstdist/so e2e actually tests the production build.
Option 2 is generally preferred for CI since it validates the actual build output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml at line 62, The CI job currently declares needs:
[build] even though the e2e tests (per playwright.config.ts) run npm run dev
against port 5173 and do not consume build artifacts; either remove needs:
[build] so e2e runs in parallel with build, or (preferred) change the e2e
workflow and playwright.config.ts to run a production preview: have the CI build
produce dist/, start the server with vite preview (serving dist/) and point
Playwright to that URL so the tests validate the production build instead of the
dev server.
| globals: { | ||
| window: 'readonly', | ||
| document: 'readonly', | ||
| console: 'readonly', | ||
| localStorage: 'readonly', | ||
| navigator: 'readonly', | ||
| setTimeout: 'readonly', | ||
| File: 'readonly', | ||
| Element: 'readonly', | ||
| HTMLInputElement: 'readonly', | ||
| React: 'readonly', | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use globals.browser instead of manually listing browser globals.
The globals package already exports a browser object containing all standard browser globals. This eliminates manual maintenance and ensures completeness.
♻️ Proposed fix
globals: {
- window: 'readonly',
- document: 'readonly',
- console: 'readonly',
- localStorage: 'readonly',
- navigator: 'readonly',
- setTimeout: 'readonly',
- File: 'readonly',
- Element: 'readonly',
- HTMLInputElement: 'readonly',
- React: 'readonly',
+ ...globals.browser,
},Apply the same change to the **/*.{ts,tsx} block (lines 61-72).
📝 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.
| globals: { | |
| window: 'readonly', | |
| document: 'readonly', | |
| console: 'readonly', | |
| localStorage: 'readonly', | |
| navigator: 'readonly', | |
| setTimeout: 'readonly', | |
| File: 'readonly', | |
| Element: 'readonly', | |
| HTMLInputElement: 'readonly', | |
| React: 'readonly', | |
| }, | |
| globals: { | |
| ...globals.browser, | |
| React: 'readonly', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eslint.config.js` around lines 34 - 45, Replace the manual browser global
list in the eslint config's globals object with the packaged browser set: import
or reference globals.browser and assign it instead of the explicit properties
(the existing globals: { window: ..., React: ... } block); also update the other
TypeScript/TSX globals block (the **/*.{ts,tsx} globals section) to use
globals.browser as well so both places rely on the canonical browser globals
export.
| @@ -24,6 +27,9 @@ | |||
| }, | |||
| "devDependencies": { | |||
| "@playwright/test": "^1.40.0", | |||
There was a problem hiding this comment.
Playwright package versions are misaligned.
@playwright/test is ^1.40.0 while playwright is ^1.59.1. These should use the same version to avoid compatibility issues between the test runner and browser binaries.
Proposed fix
- "@playwright/test": "^1.40.0",
+ "@playwright/test": "^1.59.1",Also applies to: 65-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 29, The package versions for the Playwright packages
are inconsistent: align the versions of "@playwright/test" and "playwright" so
they match (e.g., set both to the same semver like "^1.59.1" or "^1.40.0") to
avoid runner/browser binary mismatches; update the package.json entries for
"@playwright/test" and "playwright" to the same version, run a quick install to
update lockfile, and verify no other Playwright-related dependencies conflict.
| import { Footer } from './components/Footer'; | ||
| import { ScrollToTop } from './components/ScrollToTop'; | ||
| import { FAQ } from './components/FAQ'; | ||
| import { CookieConsent } from './components/CookieConsent'; |
There was a problem hiding this comment.
Use src/ absolute import for CookieConsent.
The new import should follow the project’s absolute-import convention.
Proposed fix
-import { CookieConsent } from './components/CookieConsent';
+import { CookieConsent } from 'src/components/CookieConsent';As per coding guidelines, src/**/*.{ts,tsx,js,jsx}: Use absolute imports from src/ directory.
📝 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.
| import { CookieConsent } from './components/CookieConsent'; | |
| import { CookieConsent } from 'src/components/CookieConsent'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.tsx` at line 10, Update the import in App.tsx to use the project's
absolute-import convention: replace the relative import of CookieConsent with an
absolute one from the src root (import { CookieConsent } from
'src/components/CookieConsent'), ensuring the symbol CookieConsent and its
module path are updated accordingly.
| style={{ | ||
| marginLeft: '4px', | ||
| display: 'inline-block', | ||
| shapeRendering: 'inherit', | ||
| verticalAlign: 'middle', | ||
| fill: 'currentColor', | ||
| 'path fill': 'currentColor' | ||
| }} |
There was a problem hiding this comment.
Invalid inline style property: 'path fill' is not valid CSS.
The property 'path fill': 'currentColor' on line 28 is not a valid CSS property name. This will be ignored by the browser. If the intent is to style the path's fill, it's already handled by fill: 'currentColor' on line 27.
Proposed fix
style={{
marginLeft: '4px',
display: 'inline-block',
shapeRendering: 'inherit',
verticalAlign: 'middle',
fill: 'currentColor',
- 'path fill': 'currentColor'
}}📝 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.
| style={{ | |
| marginLeft: '4px', | |
| display: 'inline-block', | |
| shapeRendering: 'inherit', | |
| verticalAlign: 'middle', | |
| fill: 'currentColor', | |
| 'path fill': 'currentColor' | |
| }} | |
| style={{ | |
| marginLeft: '4px', | |
| display: 'inline-block', | |
| shapeRendering: 'inherit', | |
| verticalAlign: 'middle', | |
| fill: 'currentColor' | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stories/Configure.mdx` around lines 22 - 29, The inline style object in
Configure.mdx contains an invalid property key 'path fill' which browsers
ignore; remove the "'path fill': 'currentColor'" entry from the style object
(leaving fill: 'currentColor') so path elements inherit the color correctly;
locate the style object where the JSX sets style={{ marginLeft: '4px', display:
'inline-block', shapeRendering: 'inherit', verticalAlign: 'middle', fill:
'currentColor', 'path fill': 'currentColor' }} and delete the 'path fill' line.
| <a | ||
| className="sb-action-link" | ||
| href="https://storybook.js.org/docs/configure/styling-and-css/?renderer=react&ref=configure" | ||
| target="_blank" | ||
| >Learn more<RightArrow /></a> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
External links with target="_blank" should include rel="noopener noreferrer".
Multiple links throughout this file open in new tabs without rel="noopener noreferrer". While modern browsers handle this better now, adding the attribute is a security best practice to prevent the new page from accessing window.opener.
This applies to all target="_blank" links in the file (lines 54, 67, 80, 103, 113, 124, 135, 145, 155, 168, 184, 195, 207, 218).
Example fix
<a
className="sb-action-link"
href="https://storybook.js.org/docs/configure/styling-and-css/?renderer=react&ref=configure"
target="_blank"
+ rel="noopener noreferrer"
>Learn more<RightArrow /></a>📝 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.
| <a | |
| className="sb-action-link" | |
| href="https://storybook.js.org/docs/configure/styling-and-css/?renderer=react&ref=configure" | |
| target="_blank" | |
| >Learn more<RightArrow /></a> | |
| <a | |
| className="sb-action-link" | |
| href="https://storybook.js.org/docs/configure/styling-and-css/?renderer=react&ref=configure" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| >Learn more<RightArrow /></a> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stories/Configure.mdx` around lines 50 - 54, All external anchor tags
that open in a new tab (those with target="_blank" such as the <a
className="sb-action-link" ...> links and other anchors in Configure.mdx) must
include rel="noopener noreferrer"; update every anchor in Configure.mdx that
uses target="_blank" (e.g., the "Learn more" link that uses RightArrow and any
similar anchors) to add rel="noopener noreferrer" to prevent the opened page
from accessing window.opener.
| @@ -0,0 +1,56 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused React import.
With the modern JSX transform, the explicit React import is unnecessary when not using React APIs directly (e.g., React.useState). This is a minor cleanup opportunity.
Optional fix
-import React from 'react';
-
+📝 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.
| import React from 'react'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stories/Header.tsx` at line 1, Remove the unused top-level import
statement "import React from 'react';" in Header.tsx since the file's Header
component does not reference React APIs directly; keep the component declaration
(Header) and any existing exports intact and run a quick lint/format to ensure
no unused-import errors remain.
| type User = { | ||
| name: string; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use interface for object types per coding guidelines.
The coding guidelines specify using interface for object types and type for unions/primitives.
♻️ Proposed fix
-type User = {
- name: string;
-};
+interface User {
+ name: string;
+}As per coding guidelines: "Use interface for object types, type for unions/primitives"
📝 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.
| type User = { | |
| name: string; | |
| }; | |
| interface User { | |
| name: string; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stories/Page.tsx` around lines 6 - 8, Replace the object type alias "type
User = { name: string }" with an equivalent interface declaration to follow the
coding guideline; locate the "User" symbol in src/stories/Page.tsx and change it
to "interface User { name: string }" wherever it's defined/used so references
remain compatible.
| const dirname = | ||
| typeof __dirname !== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url)); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider simplifying the negated condition.
Per static analysis, the negated condition can be inverted for readability.
Proposed fix
-const dirname =
- typeof __dirname !== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url));
+const dirname =
+ typeof __dirname === 'undefined'
+ ? path.dirname(fileURLToPath(import.meta.url))
+ : __dirname;📝 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.
| const dirname = | |
| typeof __dirname !== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url)); | |
| const dirname = | |
| typeof __dirname === 'undefined' | |
| ? path.dirname(fileURLToPath(import.meta.url)) | |
| : __dirname; |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 9-9: Unexpected negated condition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite.config.ts` around lines 8 - 9, The ternary initializing the dirname
constant uses a negated typeof check; update the expression to test for typeof
__dirname === 'undefined' and swap the true/false branches so the logic is not
negated. Locate the dirname declaration (symbol: dirname) that references
__dirname and fileURLToPath(import.meta.url) and replace the ternary so the
branch producing path.dirname(fileURLToPath(import.meta.url)) runs when
__dirname is undefined and __dirname is used otherwise.
There was a problem hiding this comment.
Pull request overview
This PR aims to add social sharing controls to the site footer, but it also introduces a large set of additional changes (Storybook setup, new CI E2E job, Cookie consent UI, and styling/accessibility updates) that expand the scope beyond the stated description.
Changes:
- Add
ShareButtonsto theFooterUI. - Introduce Storybook configuration + sample stories/assets, and integrate Storybook testing into Vitest.
- Add new CI workflow job for Playwright E2E and additional UI/accessibility/styling updates (skip link, animated gradient, cookie consent).
Reviewed changes
Copilot reviewed 24 out of 43 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.shims.d.ts | Adds Vitest browser-playwright type reference shim. |
| vite.config.ts | Converts Vitest config to multi-project setup and adds Storybook/Vitest browser integration. |
| src/stories/Page.tsx | Adds Storybook example Page component. |
| src/stories/Page.stories.ts | Adds Storybook story with interaction test for Page. |
| src/stories/page.css | Adds Storybook example CSS for Page. |
| src/stories/Header.tsx | Adds Storybook example Header component. |
| src/stories/Header.stories.ts | Adds Storybook stories for Header. |
| src/stories/header.css | Adds Storybook example CSS for Header. |
| src/stories/Configure.mdx | Adds Storybook MDX documentation page and embedded styles. |
| src/stories/Button.tsx | Adds Storybook example Button component. |
| src/stories/Button.stories.ts | Adds Storybook stories for example Button. |
| src/stories/button.css | Adds Storybook example CSS for Button. |
| src/stories/assets/youtube.svg | Adds Storybook asset. |
| src/stories/assets/tutorials.svg | Adds Storybook asset. |
| src/stories/assets/styling.png | Adds Storybook asset. |
| src/stories/assets/github.svg | Adds Storybook asset. |
| src/stories/assets/docs.png | Adds Storybook asset. |
| src/stories/assets/discord.svg | Adds Storybook asset. |
| src/stories/assets/context.png | Adds Storybook asset. |
| src/stories/assets/avif-test-image.avif | Adds Storybook asset (AVIF test image). |
| src/stories/assets/assets.png | Adds Storybook asset. |
| src/stories/assets/accessibility.svg | Adds Storybook asset. |
| src/index.css | Adds animated gradient CSS with reduced-motion fallback. |
| src/components/ui/Input.stories.tsx | Adds Storybook stories for app Input component. |
| src/components/ui/Container.stories.tsx | Adds Storybook stories for app Container component. |
| src/components/ui/Button.stories.tsx | Adds Storybook stories for app Button component. |
| src/components/Hero.tsx | Switches hero background styling to use the new animated gradient. |
| src/components/Footer.tsx | Renders ShareButtons in the footer. |
| src/App.tsx | Adds skip-link target attributes and renders CookieConsent. |
| package.json | Adds Storybook/Playwright/Vitest-browser deps and scripts. |
| package-lock.json | Locks new dependency graph for Storybook/Playwright/Vitest browser. |
| index.html | Adds “Skip to main content” link for accessibility. |
| eslint.config.js | Adds Storybook ESLint flat config integration. |
| .storybook/preview.ts | Adds Storybook preview parameters incl. a11y config. |
| .storybook/main.ts | Adds Storybook main config (stories/addons/framework). |
| .gitignore | Ignores Storybook build output/logs. |
| .github/workflows/ci.yml | Adds a Playwright E2E job to CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@playwright/test": "^1.40.0", | ||
| "@storybook/builder-vite": "^10.3.3", | ||
| "@storybook/react": "^10.3.3", | ||
| "@storybook/test": "^8.6.15", |
There was a problem hiding this comment.
@storybook/test is pinned to ^8.6.15 while the rest of Storybook is ^10.3.3. @storybook/test declares a peer dependency on storybook ^8.6.15 (see package-lock), so this combination is likely to produce peer-dependency conflicts or runtime incompatibilities. Align @storybook/test to the same major/minor as storybook (or remove it if storybook/test is sufficient for your setup).
| "@storybook/test": "^8.6.15", | |
| "@storybook/test": "^10.3.3", |
| "eslint-plugin-storybook": "^10.3.3", | ||
| "playwright": "^1.59.1", | ||
| "@vitest/browser-playwright": "^4.0.17" |
There was a problem hiding this comment.
playwright is added as a separate devDependency (^1.59.1) while @playwright/test is also present (^1.40.0). Having both at different versions can cause duplicate installs and mismatched browser binaries. Consider removing the direct playwright dependency or aligning both packages to the same version range.
| - run: npm ci | ||
| - run: npx playwright install --with-deps | ||
| - run: npm run e2e |
There was a problem hiding this comment.
This E2E job runs npm run e2e, but Playwright is configured for http://localhost:5173 while Vite dev server is configured for port 3000 (vite.config.ts). The job will likely fail waiting on the wrong port. Align the Playwright webServer.port/baseURL with Vite's server.port (or vice versa).
| shapeRendering: 'inherit', | ||
| verticalAlign: 'middle', | ||
| fill: 'currentColor', | ||
| 'path fill': 'currentColor' |
There was a problem hiding this comment.
In the style prop, the key 'path fill' is not a valid CSS property, so it will be ignored (and may confuse future readers). If the intent is to color the <path>, set fill="currentColor" on the <path> element (or rely on the existing fill: 'currentColor' on the <svg>).
| 'path fill': 'currentColor' |
| @@ -0,0 +1,388 @@ | |||
| import { Meta } from "@storybook/addon-docs/blocks"; | |||
|
|
|||
| import Github from "./assets/github.svg"; | |||
There was a problem hiding this comment.
Brand capitalization: Github should be GitHub (import name / asset identifier) for consistency with the official spelling.
| import Github from "./assets/github.svg"; | |
| import GitHub from "./assets/github.svg"; |
|
|
||
| <div className="sb-section sb-socials"> | ||
| <div className="sb-section-item"> | ||
| <img src={Github} alt="Github logo" className="sb-explore-image"/> |
There was a problem hiding this comment.
Brand capitalization: the alt text Github logo should be GitHub logo.
| <img src={Github} alt="Github logo" className="sb-explore-image"/> | |
| <img src={Github} alt="GitHub logo" className="sb-explore-image"/> |
| import { Footer } from './components/Footer'; | ||
| import { ScrollToTop } from './components/ScrollToTop'; | ||
| import { FAQ } from './components/FAQ'; | ||
| import { CookieConsent } from './components/CookieConsent'; |
There was a problem hiding this comment.
PR description focuses on adding footer share buttons, but this change also introduces CookieConsent and renders it in the app. Please update the PR description/acceptance criteria to cover this new user-facing feature, or split it into a separate PR to keep scope focused.
| "e2e": "playwright test", | ||
| "storybook": "storybook dev -p 6006", | ||
| "build-storybook": "storybook build" |
There was a problem hiding this comment.
This PR adds substantial Storybook infrastructure (scripts and many Storybook deps), which is not mentioned in the PR summary/acceptance criteria about footer share buttons. Consider updating the PR description to include the Storybook addition (and why it's needed), or split the Storybook setup into a dedicated PR.
Summary
Acceptance Criteria
Testing