feat(web): improve A/B test ui and ux #170
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
WalkthroughThis pull request updates various components and configurations across the web and core packages. It upgrades a dependency version and refactors UI components to include new fields (such as an Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant IP as Input Component
participant P as Projects Page Component
participant F as Fuse Search Instance
participant R as Rendered Test List
U->>IP: Enter search query
IP->>P: Update query state
P->>F: Execute fuzzy search with query
F-->>P: Return filtered tests
P->>R: Render filtered test list
sequenceDiagram
participant U as User
participant WD as Weight Dropdown
participant WP as Weight Preset Function
participant S as State Update
participant UI as Updated UI
U->>WD: Select weight preset
WD->>WP: Trigger preset calculation
WP-->>S: Return calculated weights
S->>UI: Update component state and display weights
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
apps/web/src/components/charts/Donut.tsx (1)
1-16:⚠️ Potential issueFix import statement ordering to satisfy linter.
The pipeline is failing because the import statements don't match the expected output. Please ensure imports are properly ordered according to your project's linting rules.
#!/bin/bash # Show the import sorting rules in the project grep -A 10 "import-sorting\|sort-imports" package.json || echo "No explicit import sorting rules found in package.json" # Check if there's an eslint rule for imports grep -r "import/" .eslintrc* || echo "No import rules found in eslint config"🧰 Tools
🪛 GitHub Actions: Lint and Format
[error] 2-2: Import statements differs from the output
🧹 Nitpick comments (10)
apps/web/src/components/Test/Weights.tsx (4)
2-9: Fix import ordering to pass lint and format checks.The pipeline reports a lint/format issue specifically about import statements. Run your code formatter or linting tool (e.g.,
eslint --fixorprettier --write) to ensure the imports are ordered consistently with project style guidelines.-import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, -} from "components/ui/select"; -import { Input } from "components/ui/input"; +import { Input } from "components/ui/input"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "components/ui/select";
24-31: Centralize or document color constants for consistency.Defining
WEIGHT_COLORSinline is convenient, but consider whether these color schemes might be needed elsewhere. If so, extracting them into a constants file or theming utility might help maintain consistency throughout the codebase.
101-140: Good approach for an accessible preset selection.The
PresetItemcomponent handles keyboard navigation (Enter key) for selection, which is beneficial for accessibility. However, consider also handling Space bar or other common keys for completeness.
340-382: Visual distribution bar is effective but may need clarity.Displaying both the “new” and “original” percentage visually is helpful, though the overlapping bar with a dashed border may need tooltips or a legend for clarity. Users might wonder what the shaded portion represents if they miss the revert tooltip.
apps/web/src/components/Test/CreateTestSection.tsx (6)
2-2: Fix import ordering to pass lint and format checks.Similar to the issue in
Weights.tsx, the pipeline reports a mismatch in import ordering. Use your lint/format commands to correct any ordering discrepancies automatically.🧰 Tools
🪛 GitHub Actions: Lint and Format
[error] 2-2: Import statements differs from the output
138-151: Useful preset descriptions allow quick selection.The
PRESET_DESCRIPTIONSobject is a neat way to provide prompts and examples. Consider adding i18n support if you'd like to localize these in the future.
159-165: Multiple state variables can become unwieldy.
selectedPreset,isDirty,showError,focusedIndex, etc., might be consolidated with a reducer or a more unified state management pattern if more states are added. This is optional but can improve maintainability as the component grows.
191-206: Rebalancing on remove may produce unexpected distribution.After removing an item, the code recalculates an
equalWeightfor all but one variant and assigns the remainder to the last variant. In the case of multiple consecutive removals, the distribution may appear skewed. Consider more user-driven logic or another distribution approach if you anticipate frequent variant removals.
296-339: Tooltip usage for preset selection is user-friendly.Showing a short preset description helps new or less-technical users choose the right distribution. Optionally, you can highlight the full distribution breakdown in the tooltip to provide immediate clarity.
455-463: Revert button design is consistent.Using
RotateCcwwith a tooltip is consistent with other elements. Ensure color tokens match the design system or theme.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
apps/web/package.json(1 hunks)apps/web/src/components/AddABTestModal.tsx(3 hunks)apps/web/src/components/Test/CreateTestSection.tsx(2 hunks)apps/web/src/components/Test/Metrics.tsx(1 hunks)apps/web/src/components/Test/Section.tsx(4 hunks)apps/web/src/components/Test/Serves.tsx(1 hunks)apps/web/src/components/Test/Weights.tsx(1 hunks)apps/web/src/components/charts/Donut.tsx(2 hunks)apps/web/src/components/ui/tooltip.tsx(1 hunks)apps/web/src/pages/projects/[projectId]/index.tsx(3 hunks)apps/web/src/server/trpc/router/project.ts(1 hunks)packages/core/tests/base.test.ts(0 hunks)packages/core/tests/math.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/core/tests/base.test.ts
✅ Files skipped from review due to trivial changes (2)
- apps/web/package.json
- apps/web/src/components/Test/Serves.tsx
🧰 Additional context used
🪛 GitHub Actions: Lint and Format
apps/web/src/components/charts/Donut.tsx
[error] 2-2: Import statements differs from the output
apps/web/src/components/Test/Section.tsx
[error] 7-7: Import statements differs from the output
packages/core/tests/math.test.ts
[error] 1-1: Import statements differs from the output
apps/web/src/components/ui/tooltip.tsx
[error] 1-1: Import statements differs from the output
apps/web/src/pages/projects/[projectId]/index.tsx
[error] 7-7: Import statements differs from the output
apps/web/src/components/Test/Weights.tsx
[error] 1-1: Import statements differs from the output
apps/web/src/components/Test/CreateTestSection.tsx
[error] 2-2: Import statements differs from the output
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (34)
apps/web/src/server/trpc/router/project.ts (1)
40-44: Improved test ordering ensures consistent UI display.Adding the
orderByclause for tests bycreatedAtin ascending order provides a predictable ordering of tests in the UI. This enhances user experience by displaying tests in a chronological sequence.apps/web/src/components/Test/Metrics.tsx (2)
15-15: Removed "relative" positioning from container div.The removal of the "relative" class could affect any absolutely positioned child elements that might have depended on this container for positioning. Please verify this doesn't break any layout features.
20-20: Renamed metric label from "Interactions" to "Conversions".This terminology change better represents what the metric actually measures - conversion events rather than just interactions. This improves clarity for users.
apps/web/src/components/charts/Donut.tsx (2)
60-60: Changed card content positioning from "relative" to "h-full".This change may affect the positioning context for child elements. Ensure this is consistent with your overall layout strategy.
67-74: Enhanced UX with documentation link for empty state.Adding a link to documentation when there's no data to display is a great improvement to help users understand how to get started.
apps/web/src/components/AddABTestModal.tsx (7)
14-14: Added ID field to UIVariant type for better tracking.This is a good addition that allows for unique identification of variants, which is essential for proper tracking and state management.
16-28: Improved variant initialization with unique IDs and precise weight distribution.The changes ensure each variant gets a unique ID using crypto.randomUUID() and has properly rounded weights that distribute evenly among variants.
45-50: Enhanced weight sum calculation with proper rounding.Rounding the weight sum to two decimal places helps avoid floating-point precision issues.
52-53: Added tolerance for weight sum validation.Instead of requiring exactly 100%, you now allow a small tolerance (±0.3). This is a good UX improvement to handle minor rounding differences.
63-65: Updated error message with percentage sign.Small improvement to the error message makes it clearer that weights are expressed as percentages.
72-72: Improved weight conversion with proper decimal precision.Using toFixed(3) ensures consistent decimal precision when converting percentages to decimals.
110-119: Enhanced layout with Card component wrapping.Wrapping the CreateTestSection in Card and CardContent components improves the visual hierarchy and consistency with the rest of the application design system.
apps/web/src/components/ui/tooltip.tsx (2)
14-30: Well-implemented forwardRef for TooltipContent.Good job with the TooltipContent implementation. Using React.forwardRef correctly passes the ref to the underlying component, and setting the displayName improves debugging experience. The class composition using the
cnutility keeps the styling clean and maintainable.
32-32: Appropriate component exports.The named exports are well-organized, making it easy for other components to import and use the tooltip components.
packages/core/tests/math.test.ts (2)
1-20: Good test coverage for validateWeights function.The test cases for
validateWeightscover the important scenarios including explicit weights, default equal distribution, and normalization of weights. This ensures the function behaves as expected under various inputs.🧰 Tools
🪛 GitHub Actions: Lint and Format
[error] 1-1: Import statements differs from the output
26-53: Well-structured statistical test.Good approach using a large number of iterations and a reasonable margin of error to verify the distribution. The test ensures that the
getWeightedRandomVariantfunction produces an approximately equal distribution when equal weights are provided, which is crucial for A/B testing reliability.apps/web/src/pages/projects/[projectId]/index.tsx (2)
64-86: Well-implemented search interface.The search implementation with icon positioning and responsive layout looks great. The search functionality enhances user experience by allowing quick filtering of tests.
90-97: Good use of filteredTests for conditional rendering.Using the filtered list of tests based on the search query ensures that users see relevant results, improving the overall user experience.
apps/web/src/components/Test/Section.tsx (4)
84-123: Excellent refactoring of MetricCard component.The transformation of the Card component into MetricCard with proper semantic HTML structure using CardHeader, CardTitle, and CardContent improves both the visual presentation and the semantic meaning of the UI. The tooltip implementation using Popover provides a better user experience.
157-159: Great visual enhancement with gradient and hover effects.The addition of a gradient background and hover effects improves the visual appeal and interactivity of the card component. The transition effects provide smooth feedback to user interactions.
176-201: Well-structured grid layout for metrics.The grid layout with consistent MetricCard components creates a clean, organized display of the test information. Each card has an appropriate title and helpful tooltip explanation, improving user understanding.
203-224: Nice addition of best variant information and conditional details button.Showing which variant is performing best at a glance is valuable for users, and the conditional rendering of the details button based on a feature flag is a good approach for gradually rolling out advanced features.
apps/web/src/components/Test/Weights.tsx (4)
33-84: Potential floating-point precision pitfalls in weight presets.The weight distribution functions rely heavily on
(100 - sum)adjustments andtoFixed(2). This can lead to minor floating-point imprecision. Confirm that your team is okay with approximate weights or consider storing integer values (e.g., hundredths) to avoid subtle rounding issues.
86-99: Description objects are well-defined.Maintaining
PRESET_DESCRIPTIONSas a separate map keeps the logic self-contained and avoids hardcoding textual information in multiple places.
145-152: Effective use of useMemo for initial weights.Caching the initial weights with
useMemoensures the computation only re-runs whenoptionschanges. This is a well-structured approach to avoid unnecessary recalculations on each render.
165-174: Validate new weight inputs for edge cases.The
updateWeightfunction parses the input string withparseFloat, but consider how the UI handles invalid strings (e.g., empty input) or values outside 1-100. Adding input validation or clamping can reduce potential runtime errors or unexpected states.apps/web/src/components/Test/CreateTestSection.tsx (8)
73-127: Review floating-point arithmetic in weight presets.While these preset functions work as intended, using
toFixed(2)and distributing remainders may still leave slight sums off 100%. Confirm this approach is acceptable or consider integer-based weight tracking (e.g., scaling by 100).
129-136: Global color palette is consistent with the rest of the code.
WEIGHT_COLORSis consistently defined and aligns with the approach inWeights.tsx. This maintains a cohesive look-and-feel across the UI.
167-174: Rounding weights might cause unexpected distribution.
updateWeightsalso usestoFixed(2). If certain permutations of preset usage or user input push beyond two decimals, verify you won't truncate important data. Testing with edge cases like a large number of variants or custom user inputs is advisable.
182-185: Initialize new variants with weight=1.Using 1% as a default helps ensure new variants don't get 0%. This is a logical default but confirm that your AB testing strategy includes small but non-zero initial traffic for new variants.
208-218: Ensure valid numeric input proceeding from handleWeightChange.Similar to the other file,
parseFloaton user-supplied strings might produceNaN. Consider implementing simple checks, default fallback values, or prompting for correction if an invalid input is rejected.
239-244: Reverting might lose intermediate changes not yet saved.Calling
onRevertsets the state back to the existingvariantsandtestName, but if the user typed partial changes in a field that they haven't fully committed, those changes will also be lost. This is likely desired behavior, but confirm with the team that this is the intended UX.
421-442: Distribution bar is consistent with “Weights.tsx”.Using a similar approach for the horizontal bar fosters consistency across the UI. As with the other file, consider adding instructive hints for users who might not understand partial overlay or shading when rebalancing weights.
466-473: Limit of 5 variants is enforced.Disabling the "Add Variant" button at 5 variants is straightforward. If future requirements allow more variants, consider making this dynamic or user-configurable.
| "use client"; | ||
|
|
||
| import * as React from "react"; | ||
| import * as TooltipPrimitive from "@radix-ui/react-tooltip"; | ||
|
|
There was a problem hiding this comment.
Fix import order to resolve linting failure.
The linting error is related to the import statements order. Make sure to follow the project's import order convention.
-"use client";
-
-import * as React from "react";
-import * as TooltipPrimitive from "@radix-ui/react-tooltip";
-
-import { cn } from "lib/utils";
+"use client";
+import * as React from "react";
+import * as TooltipPrimitive from "@radix-ui/react-tooltip";
+import { cn } from "@/lib/utils";📝 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.
| "use client"; | |
| import * as React from "react"; | |
| import * as TooltipPrimitive from "@radix-ui/react-tooltip"; | |
| "use client"; | |
| import * as React from "react"; | |
| import * as TooltipPrimitive from "@radix-ui/react-tooltip"; | |
| import { cn } from "@/lib/utils"; |
🧰 Tools
🪛 GitHub Actions: Lint and Format
[error] 1-1: Import statements differs from the output
| it("gets proper variants", () => { | ||
| const variants = ["a", "b", "c"]; | ||
| expect(getWeightedRandomVariant(variants, [0.3, 0.3, 0.3])).oneOf(variants); | ||
| }); |
There was a problem hiding this comment.
The .oneOf matcher appears incorrect.
The test uses expect(...).oneOf(variants) which doesn't seem to be a standard Jest matcher. Consider using expect(variants).toContain(result) instead.
- expect(getWeightedRandomVariant(variants, [0.3, 0.3, 0.3])).oneOf(variants);
+ const result = getWeightedRandomVariant(variants, [0.3, 0.3, 0.3]);
+ expect(variants).toContain(result);📝 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.
| it("gets proper variants", () => { | |
| const variants = ["a", "b", "c"]; | |
| expect(getWeightedRandomVariant(variants, [0.3, 0.3, 0.3])).oneOf(variants); | |
| }); | |
| it("gets proper variants", () => { | |
| const variants = ["a", "b", "c"]; | |
| const result = getWeightedRandomVariant(variants, [0.3, 0.3, 0.3]); | |
| expect(variants).toContain(result); | |
| }); |
packages/core/tests/math.test.ts
Outdated
| console.log({ | ||
| counts, | ||
| expectedCount, | ||
| marginOfError, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debug console.log statement.
Debug console.log statements should be removed before committing to production code. They can clutter the test output and potentially reveal sensitive information.
- console.log({
- counts,
- expectedCount,
- marginOfError,
- });📝 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.
| console.log({ | |
| counts, | |
| expectedCount, | |
| marginOfError, | |
| }); |
| import { Button } from "components/ui/button"; | ||
| import { Input } from "components/ui/input"; |
There was a problem hiding this comment.
Fix import from component directory.
According to the linting error, the import path should be updated. Maintain consistent import paths across the project.
-import { Button } from "components/ui/button";
-import { Input } from "components/ui/input";
+import { Button } from "@/components/ui/button";
+import { Input } from "@/components/ui/input";📝 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 { Button } from "components/ui/button"; | |
| import { Input } from "components/ui/input"; | |
| import { Button } from "@/components/ui/button"; | |
| import { Input } from "@/components/ui/input"; |
🧰 Tools
🪛 GitHub Actions: Lint and Format
[error] 7-7: Import statements differs from the output
| const fuse = useMemo( | ||
| () => new Fuse(data?.project.tests ?? [], { keys: ["name"] }), | ||
| [data?.project?.tests] | ||
| ); | ||
|
|
||
| const filteredTests = useMemo(() => { | ||
| if (!query) return data?.project.tests ?? []; | ||
| const results = fuse.search(query); | ||
| return results.map((result) => result.item); | ||
| }, [query, fuse.search, data?.project.tests]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential performance issue with Fuse.js initialization.
The Fuse instance is recreated whenever data?.project?.tests changes, but you're only using the array length in the dependency array, not the actual array elements. If tests are added, removed, or modified, the dependency won't track these changes correctly.
const fuse = useMemo(
() => new Fuse(data?.project.tests ?? [], { keys: ["name"] }),
- [data?.project?.tests]
+ [data?.project?.tests ?? []]
);
const filteredTests = useMemo(() => {
if (!query) return data?.project.tests ?? [];
const results = fuse.search(query);
return results.map((result) => result.item);
- }, [query, fuse.search, data?.project.tests]);
+ }, [query, fuse, data?.project?.tests]);📝 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 fuse = useMemo( | |
| () => new Fuse(data?.project.tests ?? [], { keys: ["name"] }), | |
| [data?.project?.tests] | |
| ); | |
| const filteredTests = useMemo(() => { | |
| if (!query) return data?.project.tests ?? []; | |
| const results = fuse.search(query); | |
| return results.map((result) => result.item); | |
| }, [query, fuse.search, data?.project.tests]); | |
| const fuse = useMemo( | |
| () => new Fuse(data?.project.tests ?? [], { keys: ["name"] }), | |
| [data?.project?.tests ?? []] | |
| ); | |
| const filteredTests = useMemo(() => { | |
| if (!query) return data?.project.tests ?? []; | |
| const results = fuse.search(query); | |
| return results.map((result) => result.item); | |
| }, [query, fuse, data?.project?.tests]); |
| @@ -3,15 +3,15 @@ import * as Popover from "@radix-ui/react-popover"; | |||
| import { Modal } from "components/Modal"; | |||
| import { TitleEdit } from "components/TitleEdit"; | |||
| import { Button } from "components/ui/button"; | |||
| import { Card, CardContent, CardHeader, CardTitle } from "components/ui/card"; | |||
There was a problem hiding this comment.
Fix import from component directory.
According to the linting error, the import path should be updated. Maintain consistent import paths across the project.
-import { Card, CardContent, CardHeader, CardTitle } from "components/ui/card";
+import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";📝 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 { Card, CardContent, CardHeader, CardTitle } from "components/ui/card"; | |
| import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/src/components/Test/CreateTestSection.tsx (1)
73-127: Consider extracting shared preset logic.The WeightPresets object is duplicated from the Weights component. Consider extracting this to a shared utilities file to maintain DRY principles and ensure consistency.
+// In a new file: apps/web/src/utils/weightPresets.ts +export const WeightPresets = { + "Equal Split": (count: number) => { + // ... existing implementation + }, + "Champion/Challenger": (count: number) => { + // ... existing implementation + }, + "Progressive Split": (count: number) => { + // ... existing implementation + }, +}; + +export const WEIGHT_COLORS = [ + "bg-blue-500/90 dark:bg-blue-400/90", + "bg-emerald-500/90 dark:bg-emerald-400/90", + "bg-indigo-500/90 dark:bg-indigo-400/90", + "bg-amber-500/90 dark:bg-amber-400/90", + "bg-rose-500/90 dark:bg-rose-400/90", +]; + +export const PRESET_DESCRIPTIONS = { + "Equal Split": { + description: "Distributes traffic evenly between all variants", + example: "e.g., 33.3% / 33.3% / 33.3%", + }, + "Champion/Challenger": { + description: "90% to existing variant, 10% to the new variant", + example: "e.g., 45% / 45% / 10%", + }, + "Progressive Split": { + description: "Gradually increases weight for each variant", + example: "e.g., 17% / 33% / 50%", + }, +};Then import these in both components:
-const WeightPresets = { ... } -const WEIGHT_COLORS = [ ... ] -const PRESET_DESCRIPTIONS = { ... } +import { WeightPresets, WEIGHT_COLORS, PRESET_DESCRIPTIONS } from "utils/weightPresets";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/Test/CreateTestSection.tsx(2 hunks)apps/web/src/components/Test/Weights.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint and Format
apps/web/src/components/Test/Weights.tsx
[error] 1-1: Import statements differs from the output
apps/web/src/components/Test/CreateTestSection.tsx
[error] 2-2: Import statements differs from the output
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (31)
apps/web/src/components/Test/Weights.tsx (18)
24-31: Good use of a color palette with dark mode support.The color palette is well-defined with both light and dark mode variants, ensuring consistent UI across themes.
33-84: Well-implemented weight distribution presets.The preset functions are thorough and handle edge cases well, including:
- Proper rounding to maintain exactly 100% total
- Special handling for different variant counts
- Adjustment for rounding errors by modifying the last weight
This provides users with useful predefined distribution patterns.
86-99: Clear and helpful preset descriptions.The descriptions and examples effectively communicate the purpose of each preset to users.
101-140: Enhanced selection UI with visual indicators.The PresetItem component improves usability by:
- Providing visual indicators with PieChart icons
- Including both the preset name and example distribution
- Handling keyboard navigation properly
155-162: Good state management for UI interactions.The component properly tracks:
- Current weights vs. initial weights
- Selected preset
- Modification state (isDirty)
- Input focus state
- Error display state
This enhances user interaction and feedback.
165-174: Improved weight update handler with better precision.The function now properly handles string values and ensures weights are stored with consistent decimal precision.
176-181: Well-implemented preset application.The preset application function correctly applies the selected distribution pattern and updates the UI state accordingly.
183-186: Robust weight validation check.Using a tolerance value of 0.1 is a good approach for floating-point comparisons to handle rounding errors.
189-195: Debounced error display improves UX.Delaying error messages until after user input has stabilized prevents distracting flashes during typing.
231-235: Clear revert functionality.The revert function properly resets the weights to their initial values and clears the selected preset.
238-259: Informative tooltip for original weights.The tooltip provides useful context by showing the original weight values, helping users understand what changes they've made.
261-267: Proper empty state handling.The component gracefully handles the case when no options are available by displaying an appropriate message.
270-294: Clean implementation of the distribution preset selector.The preset selector is well-structured with clear labeling and proper event handling.
296-338: Enhanced input fields with visual feedback.The weight input fields provide good visual feedback including:
- Percentage symbol indicators
- Focus state highlighting
- Proper numeric constraints
340-381: Excellent visual representation of weights.The weight distribution bar provides:
- Color-coded segments for each variant
- Visual indication of changes from original weights
- Smooth transitions for weight changes
- Proper tooltips showing percentage values
This gives users an immediate visual understanding of the weight distribution.
383-385: Good accessibility support with screen reader announcements.The screen reader announcements properly convey the current state of the weights for users relying on assistive technologies.
387-423: Well-designed validation feedback and actions area.The component:
- Clearly displays validation errors when needed
- Provides intuitive actions for reverting changes
- Disables the save button when appropriate
- Uses tooltips to provide additional context
This creates a clear and easy-to-use interface for managing weights.
2-8:❓ Verification inconclusive
Fix import order to resolve linting error.
The pipeline failure indicates an issue with the import statements. Make sure the imports are properly ordered and structured.
🏁 Script executed:
#!/bin/bash # Check what ESLint rules might be affecting import order cat .eslintrc.json | grep -i "import" || echo "No import-related rules found in .eslintrc.json"Length of output: 189
Action Required: Update import order to align with active ESLint rules.
The pipeline failure points to an import order issue, yet our check of
.eslintrc.jsondidn’t reveal any import-related rules. This suggests that the project's ESLint configuration for import ordering might be specified elsewhere (for example, in a.eslintrc.jsfile or withinpackage.json). Please verify which ESLint configuration source is active and adjust the import order accordingly so that it meets the project's standards.apps/web/src/components/Test/CreateTestSection.tsx (13)
32-39: Improved variant type with unique identifier.Adding the
idfield to variants provides stability for array operations and prevents key-related issues in React.
159-165: Good state management approach.The component maintains appropriate state variables for UI interactions, focusing, and validation feedback.
167-175: Well-implemented weight update function.The function properly updates weights with appropriate precision and maintains the dirty state.
191-206: Improved variant removal with weight rebalancing.The enhanced removal logic appropriately redistributes weights when a variant is removed, maintaining 100% total and improving user experience.
208-218: Improved weight change handler.The function now properly handles string input conversion and maintains precision through appropriate rounding.
220-231: Good weight validation approach.Using a small tolerance value for floating-point comparison and debounced error display improves the user experience.
233-244: Clear preset and revert functions.These functions provide appropriate ways to apply predefined distributions and revert changes.
266-286: Improved UI organization with card components.Using card components with proper headers and content sections creates a cleaner, more structured interface.
288-342: Well-designed variant management UI.The preset selector with tooltips provides clear options and explanations for weight distributions.
344-418: Enhanced variant input fields with visual feedback.The variant input fields now include:
- Improved focus states
- Visual indicators for percentage values
- Tooltips for delete actions
- Proper validation feedback
This creates a more intuitive and responsive interface.
421-442: Excellent visual representation of weights.The weight distribution bar provides an immediate visual understanding of the current weight distribution using consistent color coding.
444-474: Well-designed action area with appropriate controls.The component provides:
- Clear error messages
- Ability to revert changes
- Controls for adding variants with appropriate constraints
- Visual feedback for interactions
This creates an intuitive and user-friendly interface.
478-481: Good accessibility support.The screen reader announcements provide important information about pending changes and weight totals for users relying on assistive technologies.
| import { Card, CardContent, CardHeader } from "components/ui/card"; | ||
| import { Input } from "components/ui/input"; | ||
| import { cn } from "lib/utils"; | ||
| import { | ||
| Select, | ||
| SelectContent, | ||
| SelectItem, | ||
| SelectTrigger, | ||
| SelectValue, | ||
| } from "components/ui/select"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix import order to resolve linting error.
The pipeline failure indicates an issue with the import statements ordering.
🏁 Script executed:
#!/bin/bash
# Check what ESLint rules might be affecting import order
cat .eslintrc.json | grep -i "import" || echo "No import-related rules found in .eslintrc.json"Length of output: 189
Action Required: Adjust Import Order
The linting failure indicates that the import statements aren’t in the expected order. Even though no import-related rules were found in a .eslintrc.json file, the pipeline error suggests that the grouping/sorting is enforced elsewhere. In this file, the utility import for cn from "lib/utils" is placed before the UI component imports from "components/ui/select". To fix the issue, please reorder the imports so that all "components/ui" imports are grouped (and sorted alphabetically) before any other modules. For example, the correct order should be:
- Import UI components from
"components/ui/card","components/ui/input", and"components/ui/select"grouped together. - Then import utility modules like
cnfrom"lib/utils".
Revised snippet:
import { Card, CardContent, CardHeader } from "components/ui/card";
import { Input } from "components/ui/input";
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from "components/ui/select";
import { cn } from "lib/utils";This change should resolve the linting error.
🧰 Tools
🪛 GitHub Actions: Lint and Format
[error] 2-2: Import statements differs from the output
| id: crypto.randomUUID(), | ||
| name: `${DEFAULT_NEW_VARIANT_PREFIX}${maxVariantIndex + 1}`, | ||
| weight: 0, | ||
| weight: 1, | ||
| }); | ||
| updateWeights(draft); | ||
| }) | ||
| ); | ||
| setIsDirty(true); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify crypto.randomUUID() browser compatibility.
The use of crypto.randomUUID() may not be supported in all browsers. Ensure you have appropriate fallbacks for older browsers.
🌐 Web query:
What browsers support crypto.randomUUID()?
💡 Result:
crypto.randomUUID() is supported in most modern browsers but has specific version requirements and security context limitations. Here's a detailed breakdown:
Browser Support Overview
- Chrome: Full support since v92 (2021)
- Edge: Full support since v92
- Firefox: Full support since v95
- Safari:
- Desktop: v15.4+ (March 2022)
- iOS: v15.4+
- Opera: Full support since v80
- Mobile Browsers:
- Chrome for Android: v129+
- Safari iOS: v15.4+
- Samsung Internet: v25+
Key Considerations
Secure Context Requirement
The API only works in:
- HTTPS connections
- Localhost environments
- Secure development contexts[1][2][3]
Notable Limitations
// Will fail in:
// - HTTP sites
// - Safari <15.4
// - Chrome <92
const uuid = crypto.randomUUID(); Compatibility Table
| Browser | First Supported Version | Release Year |
|---|---|---|
| Chrome | 92 | 2021 |
| Firefox | 95 | 2021 |
| Safari | 15.4 | 2022 |
| Edge | 92 | 2021 |
| Opera Mobile | 80 | 2023 |
Workarounds for Older Browsers
- Use feature detection:
const uuid = crypto.randomUUID?.() || fallbackUUIDGenerator();- Implement a polyfill for unsupported environments[1][4]
- Consider the AWS Encryption SDK for cross-browser cryptography needs[6]
For projects requiring wide browser compatibility below these versions, continue using the uuid library while monitoring browser adoption statistics[4][7].
Citations:
- 1: https://dev.to/simplr_sh/ditch-the-import-why-cryptorandomuuid-is-your-new-best-friend-for-uuids-2lp3
- 2: https://developer.mozilla.org/en-US/docs/Web/API/Crypto/randomUUID
- 3: https://community.sonarsource.com/t/add-a-rule-to-flag-crypto-randomuuid-usage-in-typescript-javascript/109109
- 4: https://caniuse.com/cryptography
- 5: https://developer.mozilla.org/en-US/docs/Web/API/Crypto
- 6: https://docs.aws.amazon.com/pdfs/encryption-sdk/latest/developer-guide/encryption-sdk-developer-guide.pdf
- 7: window.crypto.randomUUID is not defined in older browsers AzureAD/microsoft-authentication-library-for-js#6784
Action Required: Implement Fallback for crypto.randomUUID() Usage
Based on our verification, crypto.randomUUID() is supported in most modern browsers (Chrome 92+, Edge 92+, Firefox 95+, Safari 15.4+, etc.) and works only in secure contexts. However, older browsers or non-secure environments will not support this API. To ensure broad compatibility, please implement a fallback mechanism for environments where crypto.randomUUID() is unavailable.
Next Steps:
- In
apps/web/src/components/Test/CreateTestSection.tsx(lines 182-189), add feature detection forcrypto.randomUUID()—for example:id: crypto.randomUUID?.() || fallbackUUIDGenerator(),
- Alternatively, consider integrating a reliable UUID generation library or polyfill to accommodate users on unsupported browsers.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/src/components/Test/CreateTestSection.tsx (1)
182-182:⚠️ Potential issueImplement fallback for crypto.randomUUID()
The use of
crypto.randomUUID()may not be supported in all browsers. You need to add a fallback mechanism to ensure compatibility.Apply this diff to implement a fallback:
- id: crypto.randomUUID(), + id: crypto.randomUUID?.() || + `variant-${Date.now()}-${Math.random().toString(36).substring(2, 9)}`,
🧹 Nitpick comments (1)
apps/web/src/components/Test/CreateTestSection.tsx (1)
478-481: Good accessibility support with screen reader announcementsThe screen reader announcements for dirty state and weight totals improve accessibility for users with disabilities. Consider adding more descriptive text for the variant actions and state changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/src/components/Test/CreateTestSection.tsx(2 hunks)apps/web/src/components/Test/Section.tsx(4 hunks)apps/web/src/components/Test/Weights.tsx(1 hunks)apps/web/src/components/charts/Donut.tsx(3 hunks)apps/web/src/components/ui/tooltip.tsx(1 hunks)apps/web/src/pages/projects/[projectId]/index.tsx(4 hunks)apps/web/src/server/queue/event.ts(0 hunks)packages/core/tests/math.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/server/queue/event.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/charts/Donut.tsx
- apps/web/src/components/ui/tooltip.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (28)
packages/core/tests/math.test.ts (2)
21-24: Fix the test assertion using standard Jest matchers.The test uses
.oneOf(variants)which is not a standard Jest matcher. This was previously flagged in a review.Apply this fix:
- expect(getWeightedRandomVariant(variants, [0.3, 0.3, 0.3])).oneOf(variants); + const result = getWeightedRandomVariant(variants, [0.3, 0.3, 0.3]); + expect(variants).toContain(result);
26-47: Well-structured statistical test for distribution.This statistical test is well implemented with:
- Sufficient iterations (10,000) for statistical significance
- Reasonable 5% margin of error
- Clear assertions for expected distribution
apps/web/src/pages/projects/[projectId]/index.tsx (4)
31-34: Fix dependency array for Fuse initialization.The current dependency will not track changes to array elements correctly, only array reference changes.
Modify to properly track test array changes:
const fuse = useMemo( () => new Fuse(data?.project.tests ?? [], { keys: ["name"] }), - [data?.project?.tests] + [data?.project?.tests ?? []] );
36-40: Fix dependency array for filtered tests.The dependency array includes
fuse.searchwhich is a method reference that doesn't change, causing potential stale closures.Update the dependency array:
const filteredTests = useMemo(() => { if (!query) return data?.project.tests ?? []; const results = fuse.search(query); return results.map((result) => result.item); - }, [query, fuse.search, data?.project.tests]); + }, [query, fuse, data?.project?.tests]);
65-75: Well-implemented search functionality with accessible UI.The search input implementation is clean with:
- Proper icon positioning
- Appropriate input attributes
- Clear visual hierarchy
89-98: Good implementation of filtered test rendering.The component now properly renders filtered tests with all necessary props passed through.
apps/web/src/components/Test/Section.tsx (4)
84-123: Well-structured MetricCard component with improved UI.The new MetricCard component provides significant improvements:
- Clear component hierarchy with Card/CardHeader/CardContent
- Accessible tooltip implementation using Popover
- Proper semantic structure with titles and content areas
157-175: Enhanced card styling with visual feedback.The implementation adds:
- Gradient background for visual interest
- Hover effects for better interaction feedback
- Improved layout for title and actions
176-201: Improved metrics display with clear information architecture.The new implementation:
- Uses consistent MetricCard components for each metric
- Provides clear, helpful tooltips explaining each metric
- Maintains good spacing and alignment
203-224: Good addition of best variant information and details button.This section adds valuable features:
- Displaying the best performing variant
- Conditional rendering of the details button based on feature flag
- Clean styling with proper spacing
apps/web/src/components/Test/Weights.tsx (6)
25-31: Good color palette choices for both light and dark modes.The color selection is thoughtful, using Tailwind colors that work well in both modes with proper opacity settings.
33-99: Well-implemented weight distribution presets.The weight preset implementations are thorough with:
- Precise calculations for different distribution strategies
- Proper handling of rounding errors to ensure 100% total
- Clear descriptions and examples for users
101-140: Nicely styled preset selection component.The PresetItem component provides:
- Visual feedback for selection state
- Keyboard accessibility for Enter key selection
- Clear information hierarchy with examples
165-174: Improved weight updating logic.The updateWeight function now:
- Properly handles string-to-number conversion
- Uses toFixed for consistent decimal places
- Marks the form as dirty for save button enablement
340-381: Excellent visual weight distribution indicator.The implementation provides:
- Clear visual representation of weight distribution
- Visual comparison between current and initial weights
- Accessible title attributes for hovering
- Proper focus indication for the currently edited weight
387-423: Improved save/revert UX with validation.The implementation includes:
- Clear error messaging for invalid weight sums
- Detailed tooltip for reverting to original weights
- Properly disabled save button when validation fails
- Screen reader announcements for accessibility
apps/web/src/components/Test/CreateTestSection.tsx (12)
1-18: The import order still needs adjustmentThe imports aren't organized according to the expected order. Based on the previous review, UI component imports should be grouped together before utility modules.
Apply this diff to fix the import order:
import { Button } from "components/ui/button"; import { Card, CardContent, CardHeader } from "components/ui/card"; import { Input } from "components/ui/input"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue, } from "components/ui/select"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger, } from "components/ui/tooltip"; import produce from "immer"; -import { cn } from "lib/utils"; import { RotateCcw } from "lucide-react"; import { PieChart, Plus, Trash2 } from "lucide-react"; +import { cn } from "lib/utils";
73-127: Well-implemented weight distribution presetsThe preset weight distribution strategies are thoughtfully implemented with careful handling of rounding errors to ensure the sum is exactly 100%. The progressive approach to handling remainders is especially well done.
The different preset strategies (Equal Split, Champion/Challenger, Progressive Split) provide valuable options for different testing scenarios, with smart edge-case handling for small variant counts.
130-151: Strong design system integration with accessibility considerationsThe color palette and descriptions are well-structured and include both light and dark mode considerations. The description object with examples helps users understand what each preset does.
159-165: Good component state managementThe state variables are well-organized and cover all necessary aspects of the component's behavior: selection state, dirty state for tracking changes, error visibility, and focus management.
167-175: Effective weight update functionThe updateWeights function correctly applies the preset weights to the variants and marks the state as dirty, indicating pending changes that need to be saved.
191-206: Excellent variant removal with weight rebalancingThe variant removal logic now properly rebalances the weights after removal, ensuring the total weight stays at 100%. This prevents users from having to manually adjust weights after removing a variant.
220-231: Good weight validation with debounced error displayThe weight validation logic properly accounts for floating-point precision issues by allowing for a small deviation (0.1%). The error display is smartly debounced with a timeout to prevent flickering during user input.
264-286: Clean UI organization with Card componentsThe UI is now better organized with card components that clearly separate different sections of the form. This improves visual hierarchy and user understanding of the interface.
289-341: Well-designed variant management header with tooltipsThe variant section header with the preset selector dropdown and tooltips provides good context and guidance for users. The tooltip descriptions help users understand what each preset does.
344-418: Enhanced variant input fields with visual feedbackThe variant input fields now provide excellent visual feedback with focus states, transitions, and clear layout. The percentage symbol placement and min/max constraints on the numeric inputs improve usability.
421-442: Effective visual weight distribution representationThe weight distribution visualization with color bands provides an intuitive representation of how traffic will be split between variants. The use of color coding and transitions creates a polished user experience.
444-477: Comprehensive error handling and action buttonsThe error messaging and action buttons section is well implemented with clear error states, revert functionality, and variant addition controls. The disable state for the Add Variant button when reaching the maximum limit prevents user error.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
@radix-ui/react-tooltipdependency.