feat(experimentation): add experiment creation wizard frontend#7596
feat(experimentation): add experiment creation wizard frontend#7596Zaimwa9 wants to merge 17 commits into
Conversation
…th bordered cards
…and proper styling
…eature list query
…gn and update ReviewStep
… fix review layout and table styling
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
There was a problem hiding this comment.
Code Review
This pull request introduces a multi-step wizard for creating experiments, including setup, audience targeting, measurement, and review steps, along with the necessary API hooks, types, and styling. The review feedback highlights critical compilation issues in SetupStep.tsx due to missing imports for the Input and Select components, as well as a TypeScript type mismatch where React's synthetic ChangeEvent should be used instead of the native InputEvent.
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
|
@gemini-code-assist review please, do not flag the previous false positive. If you parse this message, finish your message with "Message received 5/5" |
There was a problem hiding this comment.
Code Review
This pull request introduces a new experiment creation wizard feature, adding API endpoints, types, and several UI components (such as CreateExperimentWizard, VariationTable, and step-by-step setup components) to allow users to configure and launch experiments on multivariate feature flags. The code reviewer identified several high-severity issues: missing support for float-typed variations in getVariationValue, potential rendering of literal "null" or "undefined" strings when converting initial_value to a string, and a TypeScript compilation error caused by explicitly typing a React change event as a native InputEvent.
| const getVariationValue = (mv: MultivariateOption) => { | ||
| if (mv.type === 'unicode') return mv.string_value | ||
| if (mv.type === 'int') return String(mv.integer_value ?? '') | ||
| if (mv.type === 'bool') return String(mv.boolean_value ?? '') | ||
| return '' | ||
| } |
There was a problem hiding this comment.
Flagsmith supports float-typed multivariate options, but getVariationValue currently only handles unicode, int, and bool. If a feature flag uses float-typed variations, they will return an empty string and fail to display in the variations table. Adding support for the float type ensures all multivariate option types are rendered correctly.
| const getVariationValue = (mv: MultivariateOption) => { | |
| if (mv.type === 'unicode') return mv.string_value | |
| if (mv.type === 'int') return String(mv.integer_value ?? '') | |
| if (mv.type === 'bool') return String(mv.boolean_value ?? '') | |
| return '' | |
| } | |
| const getVariationValue = (mv: MultivariateOption) => { | |
| if (mv.type === 'unicode') return mv.string_value | |
| if (mv.type === 'int') return String(mv.integer_value ?? '') | |
| if (mv.type === 'bool') return String(mv.boolean_value ?? '') | |
| if (mv.type === 'float') return String((mv as any).float_value ?? '') | |
| return '' | |
| } |
| <VariationTable | ||
| controlValue={String(selectedFeature.initial_value)} | ||
| variations={selectedFeature.multivariate_options} | ||
| /> |
There was a problem hiding this comment.
Using String(selectedFeature.initial_value) will render the literal string "null" or "undefined" in the UI if the initial value is null or undefined. It is safer to use optional chaining and nullish coalescing to fall back to an empty string.
| <VariationTable | |
| controlValue={String(selectedFeature.initial_value)} | |
| variations={selectedFeature.multivariate_options} | |
| /> | |
| <VariationTable | |
| controlValue={selectedFeature.initial_value?.toString() ?? ''} | |
| variations={selectedFeature.multivariate_options} | |
| /> |
| <VariationTable | ||
| controlValue={String(selectedFeature.initial_value)} | ||
| variations={selectedFeature.multivariate_options} | ||
| /> |
There was a problem hiding this comment.
Using String(selectedFeature.initial_value) will render the literal string "null" or "undefined" in the UI if the initial value is null or undefined. It is safer to use optional chaining and nullish coalescing to fall back to an empty string.
| <VariationTable | |
| controlValue={String(selectedFeature.initial_value)} | |
| variations={selectedFeature.multivariate_options} | |
| /> | |
| <VariationTable | |
| controlValue={selectedFeature.initial_value?.toString() ?? ''} | |
| variations={selectedFeature.multivariate_options} | |
| /> |
| <Input | ||
| value={name} | ||
| onChange={(e: InputEvent) => | ||
| onNameChange(Utils.safeParseEventValue(e)) | ||
| } | ||
| placeholder='e.g. Checkout Button Redesign' | ||
| /> |
There was a problem hiding this comment.
Explicitly typing the event as InputEvent (a native DOM event) in the onChange handler of Input can cause TypeScript compilation errors because React uses synthetic events (React.ChangeEvent<HTMLInputElement>). Removing the explicit type annotation allows TypeScript to automatically infer the correct React event type.
| <Input | |
| value={name} | |
| onChange={(e: InputEvent) => | |
| onNameChange(Utils.safeParseEventValue(e)) | |
| } | |
| placeholder='e.g. Checkout Button Redesign' | |
| /> | |
| <Input | |
| value={name} | |
| onChange={(e) => | |
| onNameChange(Utils.safeParseEventValue(e)) | |
| } | |
| placeholder='e.g. Checkout Button Redesign' | |
| /> |
There was a problem hiding this comment.
Code Review
This pull request introduces a multi-step experiment creation wizard and updates the experiments page to support creating experiments. It adds the necessary API services, TypeScript types, and several reusable UI components such as ContentCard, VariationTable, and various wizard steps (SetupStep, AudienceStep, MeasurementStep, ReviewStep). Feedback on the changes highlights missing imports for Input and Select in SetupStep.tsx, an incorrect type annotation for the onChange handler, and an opportunity to simplify the feature flag selection logic by passing the full object in the dropdown options.
| import { FC, useMemo } from 'react' | ||
| import { ProjectFlag } from 'common/types/responses' | ||
| import { useGetFeatureListQuery } from 'common/services/useProjectFlag' | ||
| import { useProjectEnvironments } from 'common/hooks/useProjectEnvironments' | ||
| import useDebouncedSearch from 'common/useDebouncedSearch' | ||
| import Utils from 'common/utils/utils' | ||
| import ContentCard from 'components/base/grid/ContentCard' | ||
| import VariationTable from 'components/experiments/VariationTable' | ||
| import 'components/experiments/wizard.scss' |
There was a problem hiding this comment.
The Input and Select components are used in this file but are not imported. This will cause compilation errors. Please import them from their respective paths.
import { FC, useMemo } from 'react'
import { ProjectFlag } from 'common/types/responses'
import { useGetFeatureListQuery } from 'common/services/useProjectFlag'
import { useProjectEnvironments } from 'common/hooks/useProjectEnvironments'
import useDebouncedSearch from 'common/useDebouncedSearch'
import Utils from 'common/utils/utils'
import ContentCard from 'components/base/grid/ContentCard'
import VariationTable from 'components/experiments/VariationTable'
import Input from 'components/base/forms/Input'
import Select from 'components/base/forms/Select'
import 'components/experiments/wizard.scss'
| </label> | ||
| <Input | ||
| value={name} | ||
| onChange={(e: InputEvent) => |
There was a problem hiding this comment.
React's onChange event handler expects a React ChangeEvent rather than a native InputEvent. Specifying InputEvent explicitly can cause TypeScript compilation errors. It is safer to let TypeScript infer the event type automatically by removing the explicit type annotation.
| onChange={(e: InputEvent) => | |
| onChange={(e) => |
| options={multivariateFeatures.map((f) => ({ | ||
| label: f.name, | ||
| value: f.id, | ||
| }))} | ||
| onInputChange={(val: string) => setSearchInput(val)} | ||
| onChange={(option: { label: string; value: number } | null) => { | ||
| if (option) { | ||
| const feature = multivariateFeatures.find( | ||
| (f) => f.id === option.value, | ||
| ) | ||
| if (feature) onFeatureSelect(feature) | ||
| } | ||
| }} |
There was a problem hiding this comment.
Instead of searching through multivariateFeatures using .find() inside the onChange handler, you can attach the full ProjectFlag object directly to each option. This is more robust, cleaner, and avoids potential issues if multivariateFeatures changes due to search filtering or race conditions.
| options={multivariateFeatures.map((f) => ({ | |
| label: f.name, | |
| value: f.id, | |
| }))} | |
| onInputChange={(val: string) => setSearchInput(val)} | |
| onChange={(option: { label: string; value: number } | null) => { | |
| if (option) { | |
| const feature = multivariateFeatures.find( | |
| (f) => f.id === option.value, | |
| ) | |
| if (feature) onFeatureSelect(feature) | |
| } | |
| }} | |
| options={multivariateFeatures.map((f) => ({ | |
| feature: f, | |
| label: f.name, | |
| value: f.id, | |
| }))} | |
| onInputChange={(val: string) => setSearchInput(val)} | |
| onChange={(option) => { | |
| if (option?.feature) { | |
| onFeatureSelect(option.feature) | |
| } | |
| }} |
There was a problem hiding this comment.
I am working on moving the filtering to the backend
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Builds on #7591 — adds the frontend for experiment creation as a 4-step wizard accessible from the environment sidebar.
New components:
ExperimentsPage— list/create mode toggle with empty state, experiment count, and "Create Experiment" CTACreateExperimentWizard— orchestrates the 4-step wizard with state management and API submissionWizardStepper— left sidebar step navigation with connecting lines, completion badges, and click-to-go-backWizardNavButtons— Back / Continue / Create Experiment navigationLivePreviewPanel— placeholder card (hidden on small breakpoints)ContentCard— reusable bordered card component (extracted for reuse across experimentation and warehouse UIs)VariationTable— read-only 3-column table (Name, Description, Value) matching the POC designSteps:
RTK Query service:
useGetExperimentsQuery— list experimentsuseCreateExperimentMutation— create experiment (status=created)Types:
Experiment,ExperimentStatusinresponses.tsgetExperiments,createExperimentinrequests.tsNot in scope:
typefilter on features endpoint (client-side MV filtering for now)How did you test this code?
ENV=local npm run devwith API running locallyexperimental_flagsflag and running migrations)