Conversation
|
|
Caution Review failedThe pull request is closed. WalkthroughThe PR replaces the text-based Logo with an image-based Logo using a static Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
yamcodes
changed the title
github-actions
bot
added
www
…and remove associated Tailwind font configuration.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/www/lib/source.tsx (1)
9-26: Perfect. I've verified all the concerns. Let me summarize the findings:Verification Results:
- ✅ IconName type issue confirmed:
IconName(line 9) only includes"New"but the icon handler also accepts"Updated"(line 23-26). This creates a type-safety gap—call sites usingautocomplete<IconName>won't see "Updated" as a valid option even though the runtime handles it.- ✅ "New" is actively used: Found in
apps/www/content/docs/arkenv/how-to/reuse-schemas.mdx(line 4)- ✅ "Updated" is not used in docs: No MDX files reference the "Updated" icon, despite the handler existing
- ✅ Badge markup duplication confirmed: Lines 21-22 and 23-26 are identical except for the label text
The original review comment's assessment is correct. Here's the rewritten version:
Add
"Updated"toIconNametype union to match runtime implementationThe
IconNametype (line 9) only includes"New", but the icon handler accepts and renders"Updated"(lines 23–26). This type-implementation mismatch prevents call sites typed withautocomplete<IconName>from safely using the "Updated" value, even though it's fully supported at runtime.Update the type:
export type IconName = keyof typeof icons | "New" | "Updated";The duplicate
Badgemarkup (lines 21–22 and 23–26) could also be refactored, but that's optional.
🧹 Nitpick comments (2)
apps/www/components/page/logo.tsx (1)
1-23: Logo component matches branding goals and stays accessibleThe new Logo keeps a stable API while rendering a decorative icon plus text:
alt=""witharia-hidden="true"and adjacent “ArkEnv” text is a good accessibility pattern, andclassNameis correctly merged withcn. Reusing/assets/icon.svghere to match layout metadata and docs keeps branding consistent.If you find yourself touching the icon path often, a small shared constant for
/assets/icon.svgcould be handy, but that’s optional.apps/www/components/page/logo.test.tsx (1)
10-50: Tests track the new Logo behavior; consider small robustness & a11y-focused tweaksThe tests line up well with the new Logo structure (container + decorative img + text) and validate both the text and icon path. Using
document.querySelector("img")is reasonable here since the icon is explicitlyaria-hidden, so a11y queries won’t see it.Two optional improvements:
- Guard against nulls when using
parentElement/querySelector("img")(e.g., assert they’re not null first, or useclosest("div")) to keep the tests resilient if the markup changes and to satisfy stricter type-checking.- Add an assertion that the img remains decorative (e.g.,
expect(img).toHaveAttribute("aria-hidden", "true")and/orexpect(img).toHaveAttribute("alt", "")), which aligns with the “a11y-first” testing guidance and locks in the intended behavior more than some of the styling checks.Based on learnings, this keeps the tests focused on meaningful behavior while still catching regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/www/public/assets/icon.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/www/app/layout.config.tsx(1 hunks)apps/www/app/layout.tsx(2 hunks)apps/www/app/styles/base.css(1 hunks)apps/www/components/page/logo.test.tsx(1 hunks)apps/www/components/page/logo.tsx(2 hunks)apps/www/content/docs/arkenv/index.mdx(1 hunks)apps/www/lib/source.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Prefertypeoverinterfacefor type definitions in TypeScript
Use TypeScript 5.1+ features when appropriate
Leverageconsttype parameters for better inference in TypeScript
Use JSDoc comments for public APIs
Use tabs for indentation (configured in Biome)
Use double quotes for strings (configured in Biome)
Organize imports automatically (Biome handles this)
Avoid explicit types when TypeScript can infer them (noInferrableTypeserror)
Useas constwhere appropriate for immutable values (useAsConstAssertionerror)
Don't reassign function parameters (noParameterAssignerror)
Place default parameters last in function signatures (useDefaultParameterLasterror)
Always initialize enum values (useEnumInitializerserror)
Declare one variable per statement (useSingleVarDeclaratorerror)
Avoid unnecessary template literals (noUnusedTemplateLiteralerror)
PreferNumber.parseIntover globalparseInt(useNumberNamespaceerror)
Use kebab-case for TypeScript filenames (e.g.,create-env.ts)
Use camelCase for function names (e.g.,createEnv)
Use PascalCase for type names (e.g.,ArkEnvError)
Use UPPER_SNAKE_CASE for environment variables and constants
Include examples in JSDoc comments when helpful for public APIs
Document complex type logic with JSDoc comments
UseArkEnvErrorfor environment variable validation errors
Provide clear, actionable error messages that include the variable name and expected type
Files:
apps/www/components/page/logo.test.tsxapps/www/components/page/logo.tsxapps/www/app/layout.config.tsxapps/www/lib/source.tsxapps/www/app/layout.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
Use self-closing JSX elements (
useSelfClosingElementserror)
Files:
apps/www/components/page/logo.test.tsxapps/www/components/page/logo.tsxapps/www/app/layout.config.tsxapps/www/lib/source.tsxapps/www/app/layout.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
Co-locate tests with components:
Component.tsxnext toComponent.test.tsx
**/*.test.{ts,tsx}: Test individual functions, components, and hooks in isolation with mocked dependencies in unit tests
Keep unit tests fast with execution under 100ms per test
Co-locate unit tests with source files using naming convention:source.ts→source.test.ts
Focus component tests on public API, user behavior, state transitions, and accessibility rather than styling or implementation details
Query component tests by role, name, label, and text using accessibility-first selectors
Use Vitest as the test framework for unit and integration tests
Use Testing Library withuser-eventfor real user simulation in component testsUnit tests should be fast (< 100ms per test) and located alongside source files with
.test.tssuffix, with mocked external dependencies
Files:
apps/www/components/page/logo.test.tsx
apps/www/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/www/.cursor/rules/posthog-integration.mdc)
apps/www/**/*.{ts,tsx,js,jsx}: If using TypeScript, use an enum to store feature flag names. If using JavaScript, store feature flag names as strings to an object declared as a constant to simulate an enum. Use UPPERCASE_WITH_UNDERSCORE naming convention for enum/const object members.
If a custom property for a person or event is referenced in two or more files or two or more callsites in the same file, use an enum or const object with UPPERCASE_WITH_UNDERSCORE naming convention, similar to feature flags.
Files:
apps/www/components/page/logo.test.tsxapps/www/components/page/logo.tsxapps/www/app/layout.config.tsxapps/www/lib/source.tsxapps/www/app/layout.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Use the main functioncreateEnv(schema)or default importarkenvto create validated environment objects with TypeScript type inference from ArkType schemas
Use built-in validators (host,port,url,src/types.tswhen available instead of writing custom validation logic
Provide descriptive variable names that indicate purpose and format in environment schema definitions
Group related environment variables in logical schemas for better organization and maintainability
Provide default values for optional environment variables using ArkType's default value syntax (e.g.,"boolean = false")
Files:
apps/www/components/page/logo.test.tsxapps/www/components/page/logo.tsxapps/www/app/layout.config.tsxapps/www/lib/source.tsxapps/www/app/layout.tsx
**/*.{test,integration.test}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Vitest's
describe/itstructure with both success and failure test cases in all test files
Files:
apps/www/components/page/logo.test.tsx
🧠 Learnings (6)
📚 Learning: 2025-11-24T16:04:36.939Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-11-24T16:04:36.939Z
Learning: Applies to **/*.test.{ts,tsx} : Focus component tests on public API, user behavior, state transitions, and accessibility rather than styling or implementation details
Applied to files:
apps/www/components/page/logo.test.tsx
📚 Learning: 2025-11-24T16:04:00.957Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-24T16:04:00.957Z
Learning: Applies to **/*.test.{ts,tsx} : Co-locate tests with components: `Component.tsx` next to `Component.test.tsx`
Applied to files:
apps/www/components/page/logo.test.tsx
📚 Learning: 2025-11-29T07:59:59.399Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T07:59:59.399Z
Learning: Applies to **/packages/arkenv/**/*.{test,integration.test}.{ts,tsx} : Save and restore original `process.env` in `beforeEach`/`afterEach` hooks to avoid test pollution
Applied to files:
apps/www/components/page/logo.test.tsx
📚 Learning: 2025-11-24T16:04:36.939Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-11-24T16:04:36.939Z
Learning: Applies to **/*.test.{ts,tsx} : Query component tests by role, name, label, and text using accessibility-first selectors
Applied to files:
apps/www/components/page/logo.test.tsx
📚 Learning: 2025-11-29T08:57:24.044Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: examples/with-bun-react/.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-11-29T08:57:24.044Z
Learning: Applies to examples/with-bun-react/**/*.test.{ts,tsx,js,jsx} : Use test syntax from `bun:test` with `test` and `expect` functions
Applied to files:
apps/www/components/page/logo.test.tsx
📚 Learning: 2025-11-24T16:04:36.939Z
Learnt from: CR
Repo: yamcodes/arkenv PR: 0
File: .cursor/rules/test-patterns.mdc:0-0
Timestamp: 2025-11-24T16:04:36.939Z
Learning: Applies to **/*.integration.test.{ts,tsx} : Test how multiple units (components, hooks, functions) work together without mocking their interactions in integration tests
Applied to files:
apps/www/components/page/logo.test.tsx
🧬 Code graph analysis (4)
apps/www/components/page/logo.test.tsx (1)
apps/www/components/page/logo.tsx (1)
Logo(10-24)
apps/www/components/page/logo.tsx (1)
apps/www/lib/utils/cn.ts (1)
cn(4-6)
apps/www/app/layout.config.tsx (1)
apps/www/components/page/logo.tsx (1)
Logo(10-24)
apps/www/lib/source.tsx (1)
apps/www/components/ui/badge.tsx (1)
Badge(29-31)
⏰ 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: test-e2e (e2e)
- GitHub Check: test-e2e (a11y)
🔇 Additional comments (4)
apps/www/app/styles/base.css (1)
12-17: Code font stack correctly wired to--font-monoUsing
var(--font-mono)here lines up with the JetBrains Mono font variable defined inlayout.tsx, while keeping the existing fallbacks intact. Looks good.apps/www/app/layout.config.tsx (1)
11-16: Nav title usage matches new Logo APIUsing
<Logo className="mb-1" />keeps the layout-only spacing here and leaves typography/layout details insideLogo, which is a good separation of concerns.apps/www/content/docs/arkenv/index.mdx (1)
12-23: Icon docs and asset usage look consistent and accessibleThe new section clearly explains the icon and uses the same
/assets/icon.svgpath as the layout metadata andLogocomponent, with a descriptivealtand fixed dimensions. No issues here.apps/www/app/layout.tsx (1)
7-29: Font variables and icon metadata are wired coherentlyDefining JetBrains Mono with
variable: "--font-mono"and applyingmono.variableon<html>lines up cleanly with thecode, prefont stack inbase.css. Updatingmetadata.iconsto/assets/icon.svgkeeps the favicon/logo asset consistent across layout, logo, and docs.Also applies to: 31-36
1 participant
Closes #324
Summary by CodeRabbit
New Features
Style
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.