Skip to content

Conversation

@BYK
Copy link
Member

@BYK BYK commented Jul 2, 2025

Fixes #14221.

@BYK BYK requested a review from chargome July 2, 2025 11:04
@vercel
Copy link

vercel bot commented Jul 2, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
develop-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2025 11:53am
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2025 11:53am

@BYK BYK requested a review from Copilot July 2, 2025 11:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the onboarding flow to support sequential, uniquely numbered steps by introducing an OnboardingSteps container and an isStep flag on each OnboardingOption.

  • Exported and wired up a new OnboardingSteps component in MDX render pipeline
  • Added isStep prop & CSS class logic to OnboardingOption and created OnboardingSteps in the onboarding component
  • Wrapped existing MDX guides in OnboardingSteps and updated global CSS counters to render step numbers

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/mdxComponents.ts Imported and exposed OnboardingSteps
src/components/onboarding/index.tsx Added isStep prop, className logic, and OnboardingSteps export
docs/platforms/javascript/guides/svelte/index.mdx Wrapped Svelte guide steps in OnboardingSteps with isStep
docs/platforms/javascript/guides/react/index.mdx Updated React guide to use OnboardingSteps and isStep
app/globals.css Defined CSS counters and styles for .onboarding-step numbering
Comments suppressed due to low confidence (3)

src/components/onboarding/index.tsx:149

  • The type ReactNode is referenced but not imported; add import { ReactNode } from 'react' (or import React, { ReactNode } from 'react') at the top of this file to prevent a compile error.
  children: ReactNode;

src/components/onboarding/index.tsx:173

  • ReactNode is used again here without importing it; ensure you import it from 'react' to avoid undefined type errors.
export function OnboardingSteps({children}: {children: ReactNode}) {

docs/platforms/javascript/guides/svelte/index.mdx:183

  • The closing </OnboardingOption> for the "Verify Your Setup" step comes before the content; move this closing tag below the accompanying text so the content is correctly scoped to that step.
</OnboardingOption>

@BYK
Copy link
Member Author

BYK commented Jul 2, 2025

@sentry review

@seer-by-sentry
Copy link
Contributor

seer-by-sentry bot commented Jul 2, 2025

On it! We are reviewing the PR and will provide feedback shortly.

cursor[bot]

This comment was marked as outdated.

@seer-by-sentry
Copy link
Contributor

seer-by-sentry bot commented Jul 2, 2025

PR Description

This pull request introduces a new visual structure for onboarding steps in the documentation. The goal is to provide a clearer, more guided experience for users setting up Sentry in their applications. It replaces the previous approach of using headings with explicit step numbers with a more structured component-based approach.

Click to see more

Key Technical Changes

The key technical changes include: 1) Introduction of a new OnboardingSteps component that acts as a container for onboarding steps and manages the CSS counter. 2) Modification of the OnboardingOption component to accept an isStep prop, which applies the necessary CSS classes to render the option as a numbered step. 3) Updates to the MDX documentation files to use the new OnboardingSteps and OnboardingOption components with the isStep prop. 4) Addition of CSS rules to handle the step numbering and styling.

Architecture Decisions

The architectural decision was to encapsulate the step numbering logic within React components (OnboardingSteps and OnboardingOption) and CSS, rather than relying on manual numbering in the MDX content. This provides a more maintainable and consistent approach to managing onboarding steps across different documentation pages. The use of CSS counters allows for automatic numbering and avoids the need to manually update step numbers when steps are added or removed.

Dependencies and Interactions

This change primarily affects the documentation pages that use the OnboardingOption component. It introduces a dependency on the new OnboardingSteps component. The changes interact with the CSS stylesheet to provide the visual styling for the numbered steps. There are no direct dependencies on other parts of the system outside of the documentation components.

Risk Considerations

Potential risks include: 1) CSS conflicts with existing styles on the documentation pages. 2) Accessibility issues if the step numbering is not properly conveyed to screen readers. 3) Incorrect step numbering if the OnboardingSteps and OnboardingOption components are not used correctly in the MDX files. 4) The new components might not be fully responsive across different screen sizes.

Notable Implementation Details

A notable implementation detail is the use of CSS counters to automatically generate the step numbers. The OnboardingSteps component resets the counter, and the OnboardingOption component increments it when the isStep prop is true. The ::before pseudo-element is used to insert the step number before the heading. The data-step-prefix attribute allows for customization of the step prefix text.

@codecov
Copy link

codecov bot commented Jul 2, 2025

Bundle Report

Changes will decrease total bundle size by 3.71kB (-0.02%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sentry-docs-server-cjs 11.8MB -4.22kB (-0.04%) ⬇️
sentry-docs-client-array-push 9.8MB 510 bytes (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: sentry-docs-client-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
static/chunks/pages/_app-*.js -3 bytes 873.24kB -0.0%
static/css/*.css 230 bytes 741.44kB 0.03%
static/chunks/7750-*.js -3 bytes 415.85kB -0.0%
static/chunks/app/[[...path]]/page-*.js 286 bytes 83.99kB 0.34%
server/middleware-*.js 5.55kB 6.55kB 555.3% ⚠️
server/middleware-*.js -5.55kB 1.0kB -84.74%
static/L8zWxeitBKWeqRJl1vtXg/_buildManifest.js (New) 684 bytes 684 bytes 100.0% 🚀
static/L8zWxeitBKWeqRJl1vtXg/_ssgManifest.js (New) 77 bytes 77 bytes 100.0% 🚀
static/y5JOg2DwANtUjNforNFUV/_buildManifest.js (Deleted) -684 bytes 0 bytes -100.0% 🗑️
static/y5JOg2DwANtUjNforNFUV/_ssgManifest.js (Deleted) -77 bytes 0 bytes -100.0% 🗑️

Files in static/chunks/app/[[...path]]/page-*.js:

  • ./src/components/onboarding/index.tsx → Total Size: 15.49kB
view changes for bundle: sentry-docs-server-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
1729.js -3 bytes 1.64MB -0.0%
../instrumentation.js -3 bytes 973.36kB -0.0%
9523.js -3 bytes 949.31kB -0.0%
../app/[[...path]]/page.js.nft.json -1.64kB 691.18kB -0.24%
../app/platform-redirect/page.js.nft.json -1.64kB 691.1kB -0.24%
../app/sitemap.xml/route.js.nft.json -1.64kB 689.07kB -0.24%
../app/[[...path]]/page.js 699 bytes 592.32kB 0.12%

Files in ../app/[[...path]]/page.js:

  • ./src/mdxComponents.ts → Total Size: 3.74kB

  • ./src/components/onboarding/index.tsx → Total Size: 14.62kB

  • ./src/components/onboarding/index.tsx → Total Size: 1.79kB

App Routes Affected:

App Route Size Change Total Size Change (%)
/[[...path]] 699 bytes 3.14MB 0.02%

Co-authored-by: seer-by-sentry[bot] <157164994+seer-by-sentry[bot]@users.noreply.github.com>
Co-authored-by: seer-by-sentry[bot] <157164994+seer-by-sentry[bot]@users.noreply.github.com>
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

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

I don't think this works as expected:

e.g. the steps in https://sentry-docs-git-cursor-refactor-onboarding-options-for-u-4ebbf6.sentry.dev/platforms/javascript/guides/react/ are not correctly numbered anymore

@BYK BYK requested a review from chargome July 3, 2025 11:42
@BYK
Copy link
Member Author

BYK commented Jul 3, 2025

@chargome we should be good now. Ideally this whole <OnboardingOption> component will be refactored, getting rid of hiddenForThisOption and replacing that with platforms={['!performance']} or something similar. I'm also guessing the isStep option can be removed and moved into a CSS class by itself or a <OnboardingStep> component. That said I want to time box my changes and I think we are in a good-enough state with this patch.

Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

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

Agree that we should unify this across the guides, otherwise the isStep approach becomes confusing.

Another thing I just noticed is that the TOC on the right side does not include the steps anymore (e.g. compared to nextjs) but I don't think this is a blocker.

Thanks for looking into this!

@BYK BYK merged commit aa96509 into master Jul 3, 2025
15 checks passed
@BYK BYK deleted the cursor/refactor-onboarding-options-for-unique-titles-e008 branch July 3, 2025 12:16
@BYK
Copy link
Member Author

BYK commented Jul 3, 2025

Another thing I just noticed is that the TOC on the right side does not include the steps anymore (e.g. compared to nextjs) but I don't think this is a blocker.

I actually noticed that too but decided that it is a fix rather than a bug. This also "fixes" the anchor links as they used to have the step-1 prefix in them which was not stable (when you change onboarding options). Referring to them without the step numbers make more sense IMO.

If we want them back, we may look into a way to get the full text content of the header.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MD view does not respect onboarding options

4 participants