Skip to content

feat: onboarding prototype#7637

Draft
talissoncosta wants to merge 3 commits into
mainfrom
feat/onboarding-quickstart-7182
Draft

feat: onboarding prototype#7637
talissoncosta wants to merge 3 commits into
mainfrom
feat/onboarding-quickstart-7182

Conversation

@talissoncosta
Copy link
Copy Markdown
Contributor

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Contributes to #7544

Working prototype of the role-branched onboarding quickstart flow, gated behind the onboarding_quickstart_flow feature flag (FORCE_ON = true for local validation — flip to Utils.getFlagsmithHasFeature('onboarding_quickstart_flow') before merge).

Flow shape

  • Role selection (Engineer / Product manager / Something else) at start
  • Engineer path: SDK install snippet + first-eval polling
  • PM path: read-only integrations grid (visual capability check) + invite teammate CTA
  • Other path: skips the evaluation step, lands directly on the features page
  • Stepper label adapts per role — engineer gets "See it works", PM gets "Connect your tools"

Keyboard navigation (per #7544 hard requirement)

  • WAI-ARIA radiogroup pattern on Role + Feature pickers — arrow keys navigate and select, roving tabindex, role='radio' / aria-checked semantics
  • Enter on inputs (org / project / custom-feature) advances to next step
  • Enter on a focused option with a selection advances — saves a Tab+Enter to reach Next/Finish
  • Focus management on step change — first interactive element of the new step receives focus
  • Visible :focus-visible outline restored (scoped) — the project-wide .btn:focus-visible { box-shadow: none } in _buttons.scss hides focus for sighted keyboard users; this PR scopes a restoration to .onboarding-quickstart without touching the global rule

Other notable changes

  • CodeSnippet rewritten with minimal per-language snippets, correct package names (@flagsmith/flagsmith), and the user's chosen feature name interpolated
  • New BareButton primitive at web/components/base/BareButton.tsx for keyboard-accessible custom-styled surfaces (consumed in follow-up)
  • Per-block layout constraints (form steps narrow, evaluation step wide) — drops the legacy 1280px page cap
  • text-muted used in place of text-secondary throughout to avoid the Bootstrap \$secondary: #fae392 collision

Out of scope (per issue)

  • Backend endpoints — handleFinish still stubs the environment key
  • Activation signal — useFirstEvaluationPoll is placeholder polling; the real signal needs to pull from Influx (since flag evals go through the edge API now) and is tracked separately
  • Baseline metrics + rollout planning

How did you test this code?

  • Walked the flow manually as each role (Engineer / PM / Other)
  • Verified TypeScript + ESLint clean
  • Keyboard navigation:
    • Tab through every interactive element on every step
    • Arrow keys on Role + Feature pickers — selection follows focus
    • Enter on inputs advances; Enter on a focused selected option advances
    • Focus lands on the first interactive element of each new step on transition
    • Visible focus ring on Tab-reached buttons / radio rows
  • Confirmed the FF gate at GettingStartedSwitch works (currently FORCE_ON = true)

talissoncosta and others added 2 commits May 28, 2026 17:40
Folder/identifier rename from "onboarding-aha" to "onboarding-quickstart"
matches the design intent (fast path to first eval) rather than the
internal product term. Includes the SCSS class prefix, the feature
flag name (`onboarding_quickstart_flow`), the page component name,
and the track-event namespace.

New shape adds a role-selection step at the start (Engineer / PM /
Other) and branches the flow:

- Engineer: SDK install snippet + first-eval polling (existing)
- PM: integrations grid (visual capability check) + invite-an-engineer
- Other: skips the evaluation step entirely, drops to features page

Other changes folded in:

- CodeSnippet rebuilt with correct package names (`@Flagsmith/flagsmith`),
  minimal per-language snippets, and interpolation of the user's
  chosen feature name (not placeholder identifiers)
- Custom toggle replaced with the existing `Switch` component
- Skip button consolidated to the page header (was duplicated in each
  step footer)
- "Choose for me" buttons removed from Org + Project steps — the
  pre-fill via `useSmartDefaults` does the same job
- Per-block layout constraints (form steps narrow, evaluation step
  wide, page itself full-width) — drops the legacy 1280px page cap
- text-secondary → text-muted across the flow to avoid the Bootstrap
  `$secondary` (#fae392) collision that fails contrast on light
  surfaces
- "Aha" terminology dropped from step names — internal key
  `'evaluation'`, user-facing title "See it works"

The activation signal (real first-SDK-eval detection) is still the
polling stub from earlier — replacement signal needs to pull from
Influx, tracked separately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Keyboard accessibility:

- Enter on org / project / custom-feature inputs advances the step
- Step transition focuses the first interactive element of the new step
- RoleStep and FeatureStep now follow WAI-ARIA radiogroup pattern with
  arrow-key navigation (ArrowUp/Down/Left/Right + Home/End), roving
  tabindex, and `role='radio'` / `aria-checked` semantics
- First arrow press with no selection picks the focused option
  instead of skipping past it
- Enter on a focused option (with a valid selection) advances —
  saves a Tab + Enter to reach the Next/Finish button
- Visible `:focus-visible` outline restored within `.onboarding-quickstart`,
  scoped to override the project-wide `.btn:focus-visible { box-shadow: none }`
  rule that hides focus for sighted keyboard users

PM path content:

- "See it works" stepper label is role-aware: engineer keeps it,
  PM gets "Connect your tools"
- PM evaluation step now shows a read-only integrations grid (visual
  capability check) using the same data merge as `IntegrationSelect`
  (Flagsmith remote-config `integration_data` + `Constants.integrationSummaries`,
  deduped by title). Top 12 entries rendered as cards
- Reuse-the-whole `IntegrationSelect` was considered but discarded —
  its select-tools interaction has no downstream effect today, which
  would set wrong expectations at the AHA moment
- PM CTA copy: "Invite a teammate" (was "Invite an engineer")

New primitive:

- `BareButton` — a `<button>` reset for keyboard-accessible custom
  surfaces (card rows, custom radios, icon-only triggers). Defaults
  `type='button'`, provides a focus-visible outline. SCSS co-located.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
flagsmith-frontend-preview Ready Ready Preview, Comment May 29, 2026 8:55am
flagsmith-frontend-staging Ready Ready Preview, 💬 1 unresolved May 29, 2026 8:55am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview, Comment May 29, 2026 8:55am

Request Review

@github-actions github-actions Bot added the front-end Issue related to the React Front End Dashboard label May 28, 2026
@talissoncosta talissoncosta changed the title Onboarding: designs + prototype feat(onboarding-quickstart): role-branched prototype May 28, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new multi-step onboarding quickstart flow (AHA-first flow) gated by a feature flag, featuring custom wizard steps tailored to user roles, a slide-out progress drawer, and minimal SDK code snippets for various languages. While the implementation is clean and well-structured, several critical issues need to be addressed: unsafe JSON parsing of remote-config integration data could crash the flow for PM users; syntax errors in the generated C# code snippet will cause compiler errors; the FORCE_ON flag remains hardcoded to true; and the final redirection URL incorrectly uses display names and API keys instead of project and environment IDs.

Comment on lines +43 to +56
const pmIntegrations = useMemo(() => {
if (role !== 'pm') return []
const remote = Utils.getFlagsmithValue('integration_data')
if (typeof remote !== 'string' || !remote) {
return Constants.integrationSummaries.slice(0, 12)
}
const merged = uniqBy(
Object.values(JSON.parse(remote)).concat(
Constants.integrationSummaries,
) as IntegrationSummary[],
(v) => v.title.toLowerCase(),
)
return merged.slice(0, 12)
}, [role])
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The parsing of the remote-config value integration_data is done using JSON.parse and Object.values without any safety guards or error handling. If integration_data is malformed, null, or not a dictionary, this will throw a runtime exception and completely crash the onboarding flow for PM users.

Wrap the parsing logic in a try...catch block and validate that the parsed result is an object/array before calling Object.values or concatenating.

  const pmIntegrations = useMemo(() => {
    if (role !== 'pm') return []
    const remote = Utils.getFlagsmithValue('integration_data')
    if (typeof remote !== 'string' || !remote) {
      return Constants.integrationSummaries.slice(0, 12)
    }
    try {
      const parsed = JSON.parse(remote)
      if (!parsed || typeof parsed !== 'object') {
        return Constants.integrationSummaries.slice(0, 12)
      }
      const remoteValues = Array.isArray(parsed) ? parsed : Object.values(parsed)
      const merged = uniqBy(
        remoteValues.concat(
          Constants.integrationSummaries,
        ) as IntegrationSummary[],
        (v) => v.title?.toLowerCase(),
      )
      return merged.slice(0, 12)
    } catch (e) {
      // eslint-disable-next-line no-console
      console.error('Failed to parse integration_data', e)
      return Constants.integrationSummaries.slice(0, 12)
    }
  }, [role])

Comment on lines +81 to +86
case 'dotnet':
return `var flagsmith = new FlagsmithClient(new FlagsmithConfiguration {
EnvironmentKey = "${environmentKey}"
});
var flags = await flagsmith.GetEnvironmentFlags();
Console.WriteLine($"${featureName}: {await flags.IsFeatureEnabled(\\"${featureName}\\")}");`
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are two issues in the generated C# (dotnet) code snippet that will cause compiler errors:

  1. The first $ is escaped as \$, which outputs literal ${featureName} in the C# code. Since featureName is not a defined variable in the C# scope, this will fail to compile. It should be unescaped so that the actual feature name is interpolated at generation time.
  2. The double quotes inside the interpolation hole {} are escaped as \". In C# interpolated strings, escaping quotes inside the expression hole is a syntax error. They should be unescaped double quotes ".

Let's correct the snippet to generate valid, compilable C# code.

Suggested change
case 'dotnet':
return `var flagsmith = new FlagsmithClient(new FlagsmithConfiguration {
EnvironmentKey = "${environmentKey}"
});
var flags = await flagsmith.GetEnvironmentFlags();
Console.WriteLine($"${featureName}: {await flags.IsFeatureEnabled(\\"${featureName}\\")}");`
case 'dotnet':
return `var flagsmith = new FlagsmithClient(new FlagsmithConfiguration {
EnvironmentKey = "${environmentKey}"
});
var flags = await flagsmith.GetEnvironmentFlags();
Console.WriteLine($"${featureName}: {await flags.IsFeatureEnabled("${featureName}")}");`


// TEMP: forced on for local validation. Revert to `Utils.getFlagsmithHasFeature(...)`
// before merge so the flag actually gates the flow.
const FORCE_ON = true
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The FORCE_ON flag is hardcoded to true for local validation. As noted in the comments and PR description, this needs to be reverted to use the actual feature flag check before merging to production to avoid exposing the unfinished flow prematurely.

Suggested change
const FORCE_ON = true
const FORCE_ON = false

Comment on lines +91 to +94
const finishedDestination =
environmentKey && projectName
? `/project/${projectName}/environment/${environmentKey}/features`
: '/organisations'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The finishedDestination URL is constructed using projectName and environmentKey (which is the API key) as path parameters:
/project/${projectName}/environment/${environmentKey}/features

However, the application routes (defined in routes.js) expect numeric/alphanumeric IDs for :projectId and :environmentId (e.g., /project/:projectId/environment/:environmentId/features). Using display names or API keys in the URL path will cause routing or API fetch failures.

Please ensure that the real implementation uses the actual projectId and environmentId instead of the display name and API key.

@talissoncosta talissoncosta changed the title feat(onboarding-quickstart): role-branched prototype feat: onboarding prototype May 28, 2026
- Rename role title from "Something else" to "Other"
- Drop the evaluation step from the stepper when Other is picked —
  they never reach it, so it shouldn't appear on the timeline
- Make finishing as Other actually land on the features page;
  previously the stale environmentKey closure routed them back to
  /organisations

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request front-end Issue related to the React Front End Dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant