Core: Fix onboarding visual bugs, survey telemetry and modal dismissal by ghengeveld · Pull Request #33326 · storybookjs/storybook

ghengeveld · 2025-12-10T14:25:36Z

Closes #

What I did

Fixed some visual bugs related to the guided tour tooltips attempting to highlight elements outside the viewport, by ensuring the panel height is set correctly.
Fixed controls panel scrolling + sticky bar behavior.
Improved controls panel styling for better visual consistency.
Added "openSurvey" telemetry event for more reliable completion rate tracking.
Fixed Modal close button triggering form submission due to missing type attribute.
Fixed survey dismissal behavior to ensure modal stays closed.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

stories
unit tests
integration tests
end-to-end tests

Manual testing

Run through the onboarding until the onboarding survey, close it, reopen it (through Checklist) and close it again.

Documentation

Add or update documentation reflecting your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
- bug: Internal changes that fixes incorrect behavior.
- maintenance: User-facing maintenance tasks.
- dependencies: Upgrading (sometimes downgrading) dependencies.
- build: Internal-facing build tooling & test updates. Will not show up in release changelog.
- cleanup: Minor cleanup style change. Will not show up in release changelog.
- documentation: Documentation only changes. Will not show up in release changelog.
- feature request: Introducing a new feature.
- BREAKING CHANGE: Changes that break compatibility in some way with current major version.
- other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

New Features
- Survey automatically opens upon reaching step 6 of onboarding
- Added control over survey visibility state for better UX management
Bug Fixes
- Fixed modal close button to prevent unintended form submission
- Corrected body viewport positioning alignment
Refactor
- Simplified UI panel layout and styling consistency
- Improved onboarding state management with state-driven approach

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…ent with other panels

nx-cloud · 2025-12-10T14:26:12Z

View your CI Pipeline Execution ↗ for commit ef16fc8

Command	Status	Duration	Result
`nx run-many -t compile,check,knip,test,pretty-d...`	✅ Succeeded	9m 9s	View ↗

☁️ Nx Cloud last updated this comment at 2026-01-07 12:21:28 UTC

coderabbitai · 2025-12-10T14:32:50Z

📝 Walkthrough

Walkthrough

Refactors onboarding survey to use a prop-driven isOpen flow for IntentSurvey, replaces direct URL query manipulation with api.applyQueryParams, emits an openSurvey event when reaching step 6, and makes UI/theming/layout adjustments (panel sizing, fixed viewport body, modal close button type).

Changes

Cohort / File(s)	Summary
Onboarding / Survey `\`code/addons/onboarding/src/Onboarding.tsx``,` `code/addons/onboarding/src/Survey.tsx``	Replaced manual URL query manipulation with `api.applyQueryParams(...)`; added `isOpen` state and effect to emit `openSurvey` at step 6; call `api.setSizes({ bottomPanelHeight: 300 })` during onboarding setup; pass `isOpen={enabled}` / `isOpen` to IntentSurvey.
IntentSurvey component & stories `\`code/addons/onboarding/src/features/IntentSurvey/IntentSurvey.tsx``,` `code/addons/onboarding/src/features/IntentSurvey/IntentSurvey.stories.tsx``	Added `isOpen: boolean` prop to public props; switched modal control from `defaultOpen` to `open={isOpen}`; updated story default args to include `isOpen: true`.
Modal styling `\`code/core/src/components/components/Modal/Modal.styled.tsx``	Added `type="button"` to the Close button to prevent unintended form submission.
Controls / Addon panel UI `\`code/core/src/controls/components/ControlsPanel.tsx``,` `code/core/src/controls/components/SaveStory.tsx``,` `code/core/src/controls/manager.tsx``	Removed `ScrollArea` wrapper around `ArgsTable`; AddonWrapper styling now uses theme and conditional padding; `SaveStory` layout changed from sticky to absolute with full width; removed `hasScrollbar={false}` from `AddonPanel`.
Global theming `\`code/core/src/theming/global.ts``	Replaced `overflow: 'hidden'` body style with fixed viewport positioning (`position: 'fixed'`, `width: '100vw'`, `height: '100vh'`, `overflow: 'hidden'`).
Minor styling `\`code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx``	Adjusted `lineHeight` from `20px` to `19px`.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Onboarding
participant API
participant Survey
participant IntentSurvey
Note over Onboarding,API: Initial onboarding setup
Onboarding->>API: api.setSizes({ bottomPanelHeight: 300 })
Onboarding->>API: api.applyQueryParams({ onboarding: undefined }, { replace: true })
Note over Onboarding,Survey: Step progression to step 6
Onboarding->>Survey: update step -> step === 6
Survey->>API: emit({ type: 'openSurvey' })
API->>IntentSurvey: props.isOpen = true (drives Modal open)
IntentSurvey->>IntentSurvey: render Modal open={isOpen}
IntentSurvey-->>API: onDismiss / onComplete -> api.applyQueryParams(...) to clear onboarding

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Core: Define checklist tasks #32799 — overlaps onboarding survey flow and the same onboarding components (Onboarding/Survey/IntentSurvey) with likely related changes to survey opening logic.
Core: Significantly improve Storybook's own accessibility #32458 — modifies IntentSurvey modal open/close handling and props; directly related to the new isOpen prop and visibility control.

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4877857 and ef16fc8.

📒 Files selected for processing (2)

code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx
code/core/src/controls/components/ControlsPanel.tsx

✅ Files skipped from review due to trivial changes (1)

code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

code/core/src/controls/components/ControlsPanel.tsx

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

GitHub Check: normal
GitHub Check: nx
GitHub Check: Core Unit Tests, windows-latest

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (5)

code/core/src/controls/components/SaveStory.tsx (1)

19-27: Absolute footer bar + adjusted Info spacing are fine; ensure a stable positioning context

Switching the Save bar Container to position: 'absolute' with bottom: 0 and width: '100%', together with paddingBottom: 41 on the wrapper, is a reasonable way to keep the bar docked to the bottom of the panel while avoiding overlap with the ArgsTable. The added marginInlineStart on Info is a minor visual refinement.

One thing to double‑check: that there is always a positioned ancestor (e.g., position: 'relative' on the panel wrapper) so this bar is anchored to the controls panel rather than the viewport. If that’s not already true via AddonPanel, consider adding position: 'relative' to the immediate wrapper to make the relationship explicit.

Also applies to: 43-50

code/core/src/theming/global.ts (1)

100-108: Body‑level fixed viewport and scroll locking match the new layout model; validate cross‑browser behavior

Locking the body to position: 'fixed' with width: '100vw', height: '100vh', and overflow: 'hidden' is consistent with moving scrolling responsibilities into the app shell (panels, docs areas, etc.) and should help keep guided‑tour targeting aligned with the visible viewport.

Given this is a global change, it’s worth sanity‑checking:

Manager + docs views on small screens and mobile browsers (100vh/100vw quirks, address bar behavior).

Storybook when embedded inside another page or iframe, if that’s a supported scenario.

If you see issues there, an alternative would be to apply similar constraints to the manager root container instead of body.
code/core/src/controls/components/ControlsPanel.tsx (1)
33-43: AddonWrapper theming and padding look good; consider making it a positioned ancestor

The updated AddonWrapper styling (full height, maxHeight: '100vh', themed background, conditional bottom padding, and header styling) fits the new fixed‑viewport controls layout and ensures the Save bar doesn’t cover the last rows.

To make the Save bar’s position: 'absolute'; bottom: 0 behavior more robust, it would be safer for AddonWrapper itself to be a positioned ancestor rather than relying on upstream components:
-const AddonWrapper = styled.div<{ showSaveFromUI: boolean }>(({ showSaveFromUI, theme }) => ({
-  height: '100%',
-  maxHeight: '100vh',
+const AddonWrapper = styled.div<{ showSaveFromUI: boolean }>(({ showSaveFromUI, theme }) => ({
+  position: 'relative',
+  height: '100%',
+  maxHeight: '100vh',
   paddingBottom: showSaveFromUI ? 41 : 0,
   backgroundColor: theme.background.content,
   thead: {
     backgroundColor: theme.background.app,
     lineHeight: '19px',
   },
 }));
This guarantees the footer bar is anchored to the controls panel even if the parent panel’s positioning changes later.
code/addons/onboarding/src/Survey.tsx (1)

15-28: Local isOpen state + mount-time openSurvey event cleanly solve dismissal & telemetry

Using isOpen state (closed via disableOnboarding) to control <IntentSurvey> and emitting a one-shot { type: 'openSurvey', from: 'guide' } in useEffect gives you persistent dismissal behavior while instrumenting survey opens from the guide entry point. The dependency arrays and callback wiring look correct and should behave deterministically.

If you later introduce flows where Survey can mount with isOpen initially false, consider gating the openSurvey emit on isOpen as well so telemetry only counts actually visible surveys.

Also applies to: 51-51

code/addons/onboarding/src/Onboarding.tsx (1)

112-120: openSurvey telemetry on step 6 and fixed bottom panel height align with the intended UX fix

The new effect that emits { type: 'openSurvey', from: 'onboarding' } only when step === '6:IntentSurvey' and !hasCompletedSurvey gives you a clean telemetry signal for survey exposure from the onboarding flow without double‑counting. Adding api.setSizes({ bottomPanelHeight: 300 }) during initial onboarding setup should help keep the controls panel and tour highlights within the viewport, addressing the visual bugs mentioned in the PR description.

If you later want to preserve user‑customized panel sizes, consider capturing the previous sizes before calling setSizes and restoring them in a cleanup effect when onboarding is disabled or unmounted.

Also applies to: 122-129

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6127326 and 4877857.

📒 Files selected for processing (9)

code/addons/onboarding/src/Onboarding.tsx (3 hunks)
code/addons/onboarding/src/Survey.tsx (3 hunks)
code/addons/onboarding/src/features/IntentSurvey/IntentSurvey.stories.tsx (1 hunks)
code/addons/onboarding/src/features/IntentSurvey/IntentSurvey.tsx (2 hunks)
code/core/src/components/components/Modal/Modal.styled.tsx (1 hunks)
code/core/src/controls/components/ControlsPanel.tsx (2 hunks)
code/core/src/controls/components/SaveStory.tsx (2 hunks)
code/core/src/controls/manager.tsx (1 hunks)
code/core/src/theming/global.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{js,jsx,json,html,ts,tsx,mjs}