Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough## Walkthrough
This change updates multiple files across the application to import the `ControlCloud` component and the `transformStructuredOutputToFilters` utility from the external `@unkey/ui` package instead of local paths. It also introduces new documentation, example, utility, type, and test files to the internal UI package, expanding its filter and keyboard shortcut handling capabilities.
## Changes
| Files/Paths | Change Summary |
|-----------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------|
| `apps/dashboard/app/(app)/*/components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/*/_components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/audit/components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/authorization/permissions/components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/authorization/roles/components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/logs/components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/ratelimits/*/logs/components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/ratelimits/*/_overview/components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/ratelimits/_components/control-cloud/index.tsx`,<br>`apps/dashboard/app/(app)/settings/root-keys/components/control-cloud/index.tsx` | Changed import source of `ControlCloud` from local to `@unkey/ui`. |
| `apps/dashboard/app/(app)/*/components/controls/components/logs-search/index.tsx`,<br>`apps/dashboard/app/(app)/*/_components/components/controls/components/logs-search/index.tsx`,<br>`apps/dashboard/app/(app)/audit/components/controls/components/logs-search/index.tsx`,<br>`apps/dashboard/app/(app)/authorization/permissions/components/controls/components/logs-search/index.tsx`,<br>`apps/dashboard/app/(app)/authorization/roles/components/controls/components/logs-search/index.tsx`,<br>`apps/dashboard/app/(app)/logs/components/controls/components/logs-search/index.tsx`,<br>`apps/dashboard/app/(app)/ratelimits/*/logs/components/controls/components/logs-search/index.tsx`,<br>`apps/dashboard/app/(app)/ratelimits/*/_overview/components/controls/components/logs-search/index.tsx`,<br>`apps/dashboard/app/(app)/ratelimits/_components/components/controls/components/logs-search/index.tsx` | Changed import source of `transformStructuredOutputToFilters` from local to `@unkey/ui`. |
| `apps/engineering/content/design/components/filter/control-cloud.examples.tsx` | Added new example file with multiple exported React components demonstrating `ControlCloud` usage. |
| `apps/engineering/content/design/components/filter/control-cloud.mdx` | Added new documentation file for the `ControlCloud` component, including usage, props, and best practices. |
| `internal/ui/package.json` | Added dependencies: `nuqs@1.17.6` and `zod@3.23.5`; moved `tailwindcss-animate` to devDependencies. |
| `internal/ui/src/components/logs/control-cloud/control-pill.tsx`,<br>`internal/ui/src/components/logs/control-cloud/index.tsx` | Refactored imports for clarity; improved event typing; added `"use client"` directive. |
| `internal/ui/src/index.ts` | Added exports for `control-cloud` and `transform-structured-output-filter-format` modules. |
| `internal/ui/src/lib/utils.ts` | Added utility functions: `isBrowser`, `debounce`, `throttle`, `getTimestampFromRelative`, `processTimeFilters`, `getBaseUrl`, `deepMerge`. |
| `internal/ui/src/hooks/use-keyboard-shortcut.tsx` | Added new hook for global keyboard shortcut handling and related types/utilities. |
| `internal/ui/src/hooks/use-keyboard-shortcut.test.tsx` | Added comprehensive tests for `useKeyboardShortcut` hook and shortcut string parsing. |
| `internal/ui/src/validation/filter.types.ts` | Added new types for filter operators, field configs, filter values, and related Zod schemas. |
| `internal/ui/src/validation/utils/nuqs-parsers.ts` | Added new parsers for relative time, filter value arrays, and sort arrays for URL query handling. |
| `internal/ui/src/validation/utils/structured-output-schema-generator.ts` | Added utilities for generating and validating structured filter output schemas using Zod. |
| `internal/ui/src/validation/utils/transform-structured-output-filter-format.ts` | Added utility to transform structured output into internal filter format. |
| `internal/ui/src/validation/utils/type-guards.ts` | Added type guard functions for distinguishing string and number field configs. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant AppComponent
participant @unkey/ui
AppComponent->>@unkey/ui: import { ControlCloud, transformStructuredOutputToFilters }
AppComponent->>ControlCloud: Render with filters, handlers, formatting props
AppComponent->>transformStructuredOutputToFilters: Convert structured output to filter format
Note right of AppComponent: All logic and UI now sourced from @unkey/uiPossibly related PRs
Suggested reviewers
Learnt from: ogzhanolguncu Learnt from: p6l-richard Learnt from: ogzhanolguncu Learnt from: p6l-richard Learnt from: MichaelUnkey Learnt from: p6l-richard Learnt from: p6l-richard Learnt from: ogzhanolguncu Learnt from: p6l-richard Learnt from: ogzhanolguncu Learnt from: ogzhanolguncu Learnt from: chronark Learnt from: mcstepp Learnt from: mcstepp Learnt from: mcstepp Learnt from: ogzhanolguncu Learnt from: p6l-richard Learnt from: p6l-richard Learnt from: ogzhanolguncu Learnt from: ogzhanolguncu |
MichaelUnkey
requested review from
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (14)
apps/dashboard/app/(app)/apis/[apiId]/_overview/components/control-cloud/index.tsx (1)
2-2: Same import change as noted earlier – see the first comment for the required verifications.apps/dashboard/app/(app)/audit/components/control-cloud/index.tsx (1)
2-2: Repeats the@unkey/uiimport switch; the considerations from the first file apply here as well.apps/dashboard/app/(app)/authorization/roles/components/control-cloud/index.tsx (1)
2-2: Same import migration; please refer to the initial verification checklist.apps/dashboard/app/(app)/logs/components/control-cloud/index.tsx (1)
2-2: Import updated to@unkey/ui; identical concerns as previously raised.apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/control-cloud/index.tsx (1)
2-2: Same import consolidation as earlier commentNothing else changed – rely on previous note regarding dependency declaration.
apps/dashboard/app/(app)/apis/_components/control-cloud/index.tsx (1)
2-2: Consistent switch to@unkey/uiMatches the pattern; no additional remarks beyond the first comment.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/control-cloud/index.tsx (1)
2-2: Consistent switch to@unkey/uiSame feedback as earlier: ensure the package is declared in the relevant
package.json.apps/dashboard/app/(app)/ratelimits/_components/control-cloud/index.tsx (1)
2-2: Consistent switch to@unkey/uiImport path update looks correct. Refer to the initial comment for dependency-presence verification.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-search/index.tsx (1)
4-4: Same verification needed for Keys Logs-SearchPlease run the export check above for this usage path as well to avoid runtime import errors.
apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-search/index.tsx (1)
4-4: Same verification needed for Overview Logs-SearchImport relies on the same re-export. Verify package version and symbol presence.
apps/dashboard/app/(app)/authorization/permissions/components/controls/components/logs-search/index.tsx (1)
4-4: Same consideration as earlier comment about package-root importSee the advice on the first file regarding sub-path imports and leftover references.
apps/dashboard/app/(app)/logs/components/controls/components/logs-search/index.tsx (1)
5-5: Same consideration as earlier comment about package-root importSee the advice on the first file regarding sub-path imports and leftover references.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-search/index.tsx (1)
4-4: Same consideration as earlier comment about package-root importSee the advice on the first file regarding sub-path imports and leftover references.
apps/dashboard/app/(app)/audit/components/controls/components/logs-search/index.tsx (1)
4-4: Same consideration as earlier comment about package-root importSee the advice on the first file regarding sub-path imports and leftover references.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
apps/dashboard/app/(app)/apis/[apiId]/_overview/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/_components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/audit/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/authorization/permissions/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/authorization/permissions/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/authorization/roles/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/authorization/roles/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/logs/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/logs/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/control-cloud/index.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/controls/components/logs-search/index.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/_components/control-cloud/index.tsx(1 hunks)apps/engineering/content/design/components/filter/control-cloud.examples.tsx(1 hunks)apps/engineering/content/design/components/filter/control-cloud.mdx(1 hunks)internal/ui/package.json(1 hunks)internal/ui/src/components/logs/control-cloud/control-pill.tsx(2 hunks)internal/ui/src/components/logs/control-cloud/index.tsx(2 hunks)internal/ui/src/hooks/use-keyboard-shortcut.test.tsx(1 hunks)internal/ui/src/hooks/use-keyboard-shortcut.tsx(1 hunks)internal/ui/src/index.ts(1 hunks)internal/ui/src/lib/utils.ts(1 hunks)internal/ui/src/validation/filter.types.ts(1 hunks)internal/ui/src/validation/utils/nuqs-parsers.ts(1 hunks)internal/ui/src/validation/utils/structured-output-schema-generator.ts(1 hunks)internal/ui/src/validation/utils/transform-structured-output-filter-format.ts(1 hunks)internal/ui/src/validation/utils/type-guards.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/engineering/content/design/components/filter/control-cloud.mdx
[uncategorized] ~125-~125: Loose punctuation mark.
Context: ...us ## Keyboard Shortcuts - ⌥+⇧+D: Clear all filters and set a default tim...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (25)
apps/dashboard/app/(app)/authorization/permissions/components/control-cloud/index.tsx (1)
2-2: ```shell
#!/bin/bash
set -e1. Locate the @unkey/ui package.json anywhere in the repo
UI_PKG_JSON=$(fd -e json package.json . | xargs -r rg -l '"name"\s*:\s*"@unkey/ui"')
if [ -z "$UI_PKG_JSON" ]; then
echo "❌ Could not find @unkey/ui package.json in the workspace."
exit 1
fi
echo "✅ Found UI package.json at: $UI_PKG_JSON"
UI_PKG_DIR=$(dirname "$UI_PKG_JSON")
echo "UI package directory: $UI_PKG_DIR"2. Inspect the public barrel for ControlCloud export
for BARREL in "$UI_PKG_DIR"/src/index.ts "$UI_PKG_DIR"/src/index.tsx; do
if [ -f "$BARREL" ]; then
echo "🔍 Inspecting barrel: $BARREL"
rg -n "export\s+.*ControlCloud" "$BARREL" || echo "⚠️ No ControlCloud export found in $BARREL"
fi
done3. Verify @unkey/ui is declared in each workspace package.json
echo "🔍 Checking workspace package.json files for @unkey/ui dependency"
fd -e json package.json . | xargs -r rg -n '"@unkey/ui"' || echo "⚠️ @unkey/ui is not declared in at least one package.json"4. Check tsconfig for any local alias to control-cloud
echo "🔍 Searching tsconfig for '@/components/logs/control-cloud' alias"
rg -n '"@/components/logs/control-cloud"' tsconfig*.json && echo "❌ Local alias found" || echo "✅ No leftover alias"</details> <details> <summary>apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/control-cloud/index.tsx (1)</summary> `2-2`: Let’s try a simpler check across all package.json files: ```shell #!/usr/bin/env bash # Search every package.json for a declaration of @unkey/ui grep -R '"@unkey\/ui"[[:space:]]*:' -n --include="package.json" .apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/control-cloud/index.tsx (1)
2-2: Ensure the new package export matches the previous contractThe local implementation was replaced with
@unkey/ui.
Please verify that the external package still exportsControlCloudwith the same prop signature (especiallyhistoricalWindow,filters,updateFilters,removeFilter,formatFieldName). A silent mismatch will compile but break at runtime.#!/bin/bash # Confirm that ControlCloud is exported from the package and review its props. fd --extension ts 'index.ts' $(npm root)/@unkey/ui | xargs -I{} sed -n '1,120p' {}internal/ui/package.json (1)
29-29: Let’s re-run a comprehensive search for any runtime references tonuqsin the UI package:#!/bin/bash # 1. ES6 imports echo "🔍 Searching for ES6 imports of nuqs..." rg -n "import.*['\"]nuqs['\"]" internal/ui/src || echo "No ES6 imports found" # 2. CommonJS requires echo -e "\n🔍 Searching for CommonJS requires of nuqs..." rg -n "require\(['\"]nuqs['\"]\)" internal/ui/src || echo "No requires found" # 3. Any direct references echo -e "\n🔍 Searching for any other references to nuqs..." rg -n "nuqs" internal/ui/src || echo "No references found"apps/dashboard/app/(app)/authorization/roles/components/controls/components/logs-search/index.tsx (1)
4-4: ```bash
#!/bin/bash1. Search entire repo for definition or references of transformStructuredOutputToFilters
grep -R "transformStructuredOutputToFilters" -n .
2. Check package.json files for @unkey/ui dependency and its version
grep -R ""@unkey/ui"" -n .
</details> <details> <summary>apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/controls/components/logs-search/index.tsx (1)</summary> `4-4`: Let’s verify whether the `@unkey/ui` package exports support deep imports for this util: ```shell #!/bin/bash # Show package entry and export mapping grep -n '"main"' -n internal/ui/package.json || echo "No main field found" grep -n '"exports"' -n internal/ui/package.json || echo "No exports field found"apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/controls/components/logs-search/index.tsx (1)
4-4: LGTM: Clean import refactor to external package.The import source change from local path to
@unkey/uipackage is consistent with the PR objective to centralize UI components. The function usage on line 25 remains unchanged, maintaining functional compatibility.internal/ui/src/index.ts (1)
4-5: LGTM: New exports enable component centralization.The wildcard exports for
control-cloudandtransform-structured-output-filter-formatproperly expose the components and utilities that are being imported by external packages. The placement is consistent with the existing export structure.internal/ui/src/components/logs/control-cloud/control-pill.tsx (2)
1-9: LGTM: Improved import organization and type safety.The changes enhance the code quality:
- Namespace React import with explanatory biome ignore comment is appropriate
- Relative import path updates align with the new module structure
- Explicit React hooks imports improve clarity
40-40: LGTM: Enhanced type specificity for event handler.The type annotation change from generic
KeyboardEventtoReact.KeyboardEvent<HTMLButtonElement>improves type safety by being more specific about the element type and using React's synthetic event system.internal/ui/src/components/logs/control-cloud/index.tsx (2)
1-7: LGTM: Proper client-side component setup and import organization.The changes follow React best practices:
"use client"directive correctly marks this as a client-side component- Namespace React import with explanatory biome ignore comment is consistent
- Relative import path updates properly reflect the new module structure
64-64: LGTM: Improved type safety for keyboard event handling.The event handler type refinement to
React.KeyboardEvent<HTMLDivElement>provides better type safety by specifying the exact element type that will receive the keyboard events, aligning with React's synthetic event system.internal/ui/src/validation/utils/type-guards.ts (1)
1-9: LGTM: Clean and well-structured type guard implementation.The type guard functions follow TypeScript best practices:
- Clear, descriptive function names that convey their purpose
- Proper type guard syntax using
config is Typereturn type annotation- Simple discriminant logic based on the
typeproperty- Good modular separation in a dedicated utilities file
internal/ui/src/validation/utils/transform-structured-output-filter-format.ts (1)
18-61: Well-implemented filter transformation utility!The function correctly handles merging and deduplication of filters using an efficient Set-based approach. Good use of TypeScript generics for type safety.
internal/ui/src/hooks/use-keyboard-shortcut.test.tsx (1)
1-493: Excellent test coverage!Comprehensive test suite covering all major scenarios including edge cases, error conditions, and lifecycle management. Well-structured with clear test descriptions.
apps/engineering/content/design/components/filter/control-cloud.mdx (1)
1-198: Comprehensive documentation!Well-structured documentation with clear examples, keyboard navigation guide, and accessibility considerations. The formatting is correct throughout.
internal/ui/src/lib/utils.ts (5)
8-25: LGTM!The
isBrowserconstant anddebouncefunction are well-implemented with proper type safety and cleanup handling.
27-132: LGTM!Comprehensive throttle implementation with proper support for leading/trailing edges, cancel, and flush methods. The TypeScript generics are well-structured.
134-159: LGTM!The relative time parser correctly validates input format and converts time units to milliseconds. Good error messaging with examples.
173-186: LGTM!Standard implementation for environment-aware base URL detection, commonly used in Next.js applications.
188-215: Consider handling circular references.The deepMerge function could encounter stack overflow with circular references in the input objects.
Consider adding circular reference detection or documenting this limitation. Would you like me to provide an implementation that handles circular references?
apps/engineering/content/design/components/filter/control-cloud.examples.tsx (1)
1-29: LGTM!Good setup with local type definitions and helper function for creating mock data. This makes the examples self-contained and easy to understand.
internal/ui/src/validation/utils/nuqs-parsers.ts (2)
5-25: LGTM!The relative time parser properly leverages the existing validation function and handles errors gracefully.
27-57: LGTM!Well-implemented generic parser with proper handling of values containing colons using regex split pattern.
internal/ui/src/validation/filter.types.ts (1)
1-62: LGTM!Well-structured type definitions with good use of generics and composition. The separation between base configs and specialized variants (number/string/sortable) provides excellent flexibility.
apps/engineering/content/design/components/filter/control-cloud.examples.tsx
Outdated
Show resolved
Hide resolved
apps/engineering/content/design/components/filter/control-cloud.examples.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
internal/ui/src/validation/utils/nuqs-parsers.ts (1)
67-96: Consistency issue from previous review has been resolved.Good improvement! The
parseAsSortArrayparser now returns an empty array on parse failure (lines 70, 87), which aligns with the behavior ofparseAsFilterValueArray. This provides consistent error handling across all parsers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/engineering/content/design/components/filter/control-cloud.examples.tsx(1 hunks)internal/ui/package.json(2 hunks)internal/ui/src/hooks/use-keyboard-shortcut.tsx(1 hunks)internal/ui/src/lib/utils.ts(1 hunks)internal/ui/src/validation/utils/nuqs-parsers.ts(1 hunks)internal/ui/src/validation/utils/structured-output-schema-generator.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/ui/src/validation/utils/structured-output-schema-generator.ts (2)
internal/ui/src/validation/filter.types.ts (1)
FieldConfig(40-42)internal/ui/src/validation/utils/type-guards.ts (2)
isStringConfig(7-9)isNumberConfig(4-6)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test Go API Local / Test (Shard 6/8)
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: autofix
🔇 Additional comments (6)
internal/ui/package.json (1)
30-30: LGTM! Dependencies are correctly placed.The new runtime dependencies
nuqsandzodare appropriately added to support the new validation and parsing utilities. Also,tailwindcss-animatehas been correctly moved to devDependencies as suggested in the previous review.Also applies to: 34-34
apps/engineering/content/design/components/filter/control-cloud.examples.tsx (1)
1-272: Well-structured examples with comprehensive coverage!The examples effectively demonstrate various use cases of the ControlCloud component:
- Proper TypeScript typing with imported
FilterOperatortype- Cross-platform keyboard shortcut detection
- Clear demonstration of different filter types and states
- Interactive example for dynamic filter management
internal/ui/src/validation/utils/nuqs-parsers.ts (4)
1-4: Import statements look appropriate.The imports are well-organized, bringing in the necessary Parser type, utility function, and type definitions.
5-25: parseAsRelativeTime implementation is solid.The parser correctly validates relative time strings using the utility function and handles null inputs gracefully. The error handling with try-catch ensures robustness.
29-57: parseAsFilterValueArray parser handles complex cases well.The use of
split(/:(.+)/)with a capturing group is clever - it correctly splits on the first colon only, allowing filter values to contain colons. The operator validation and error handling are appropriate.
59-65: Type definitions are well-structured.The
SortDirectionandSortUrlValuetypes provide good type safety with appropriate constraints usingconstassertions and generics.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
apps/engineering/content/design/components/filter/control-cloud.examples.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Go API Local / Test (Shard 6/8)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/engineering/content/design/components/filter/control-cloud.examples.tsx (1)
271-273: Keyboard shortcut implementation looks good.Great improvement! The platform detection properly addresses the previous review concern about Mac-specific keyboard shortcuts by showing appropriate shortcuts for different platforms.
apps/engineering/content/design/components/filter/control-cloud.examples.tsx
Outdated
Show resolved
Hide resolved
apps/engineering/content/design/components/filter/control-cloud.examples.tsx
Show resolved
Hide resolved
apps/engineering/content/design/components/filter/control-cloud.examples.tsx
Outdated
Show resolved
Hide resolved
…/unkey into move-control-cloud-to-ui
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
internal/ui/src/validation/utils/structured-output-schema-generator.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2825
File: apps/dashboard/app/(app)/logs-v2/components/controls/components/logs-datetime/index.tsx:0-0
Timestamp: 2025-01-30T20:38:00.058Z
Learning: In the logs dashboard, keyboard shortcuts that toggle UI elements (like popovers) should be implemented in the component that owns the state being toggled, not in the presentational wrapper components. For example, the 'T' shortcut for toggling the datetime filter is implemented in DatetimePopover, not in LogsDateTime.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/ui/button.tsx:39-46
Timestamp: 2024-10-23T16:33:02.143Z
Learning: In `apps/www/components/ui/` directory, components are based on shadcn UI patterns, including their typing conventions.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3115
File: apps/dashboard/components/logs/checkbox/filters-popover.tsx:33-55
Timestamp: 2025-04-24T14:34:30.621Z
Learning: In the ShortcutActivator component within filters-popover.tsx, the purpose is to track keys separately for each filter item, providing a registration mechanism for shortcuts passed to it rather than enforcing specific key combinations like option+shift+key.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/search.tsx:16-20
Timestamp: 2024-10-23T16:21:47.395Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, refactoring type definitions into an interface is not necessary at this time.
internal/ui/src/validation/utils/structured-output-schema-generator.ts (4)
Learnt from: MichaelUnkey
PR: unkeyed/unkey#2810
File: internal/ui/src/components/date-time/components/time-split.tsx:10-14
Timestamp: 2025-01-22T16:51:59.978Z
Learning: The DateTime component in internal/ui/src/components/date-time/components/time-split.tsx already includes sufficient validation through handleChange and handleBlur functions, making additional runtime validation unnecessary.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/search.tsx:16-20
Timestamp: 2024-10-23T16:21:47.395Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, refactoring type definitions into an interface is not necessary at this time.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/search.tsx:41-57
Timestamp: 2024-10-23T16:19:42.049Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, adding error handling and loading states to the results list is not necessary.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Go API Local / Test (Shard 6/8)
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
internal/ui/src/validation/utils/structured-output-schema-generator.ts (2)
1-3: Imports look appropriate for the functionality.The imports correctly include Zod for schema validation, type definitions, and type guards needed for the validation logic.
92-111: Good improvement: strict validation with proper type checking.The
validateFieldValuefunction correctly returnsfalsefor type mismatches (line 110), addressing the previous concern about being too permissive. The logic properly handles both string and number configurations with appropriate validation.
internal/ui/src/validation/utils/structured-output-schema-generator.ts
Outdated
Show resolved
Hide resolved
internal/ui/src/validation/utils/structured-output-schema-generator.ts
Outdated
Show resolved
Hide resolved
2 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores