-
Notifications
You must be signed in to change notification settings - Fork 1
[DEV-2984] Add step rendering #1734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 68db798 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
# Conflicts: # .changeset/fuzzy-papayas-raise.md # apps/nextjs-website/src/components/molecules/MarkdownPart/MarkdownPart.tsx # apps/nextjs-website/src/components/organisms/GitBookContent/GitBookContent.tsx # apps/nextjs-website/src/components/organisms/GitBookContent/components/Heading.tsx # apps/nextjs-website/src/lib/cmsApi.ts # apps/nextjs-website/src/lib/strapi/types/part.ts
apps/nextjs-website/src/components/organisms/GitBookContent/components/Step.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should revert this change
export const step: Schema = { | ||
render: 'Step', | ||
// Defines the content model for what's inside a stepper | ||
children: ['paragraph', 'list', 'code', 'heading', 'blockquote'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for other content models
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds GitBook Stepper and Step Markdoc schema plus React rendering components to enable step-based content in documentation.
- Introduces Markdoc schemas (stepper, step) and corresponding React components (Stepper, Step)
- Extends rendering/parse pipelines to recognize new tags
- (Potentially accidental) temporary change returning a hardcoded string in S3 download helper
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
packages/gitbook-docs/src/renderContent.ts | Registers Stepper and Step in rendering components interface |
packages/gitbook-docs/src/parseContent.ts | Adds stepper and step tags to Markdoc schema registry |
packages/gitbook-docs/src/markdoc/schema/stepper.ts | Defines Markdoc schemas and transform logic for stepper/step |
packages/gitbook-docs/src/helpers/s3Bucket.helper.ts | Alters file download to return hardcoded string (likely unintended) |
apps/nextjs-website/src/components/templates/GitBookTemplate/GitBookTemplate.tsx | Minor import reordering (no functional change) |
apps/nextjs-website/src/components/organisms/GitBookContent/components/Stepper.tsx | New React Stepper component injecting step numbers |
apps/nextjs-website/src/components/organisms/GitBookContent/components/Step.tsx | New React Step component formatting numbered heading and body |
apps/nextjs-website/src/components/organisms/GitBookContent/GitBookContent.tsx | Wires Stepper/Step into component map |
apps/nextjs-website/src/components/molecules/MarkdownPart/MarkdownPart.tsx | Import order adjustment (no functional change) |
.changeset/open-guests-think.md | Declares minor version bumps for new feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
export const stepper: Schema = { | ||
render: 'Stepper', | ||
// This should be 'step', listing the allowed child tags. | ||
children: ['Step'], |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The children array lists 'Step' (capitalized) but the schema tag key you registered is 'step'; Markdoc expects the tag names (schema keys), so this should be ['step'] to allow step tags to be parsed as children. The inline comment already indicates the intended value.
children: ['Step'], | |
children: ['step'], |
Copilot uses AI. Check for mistakes.
for (const child of node.children) { | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore | ||
if (child.type === 'Step') { | ||
// Mutate the child node to add the index to its attributes. | ||
// eslint-disable-next-line functional/immutable-data,functional/no-expression-statements | ||
child.attributes.index = stepIndex; | ||
// eslint-disable-next-line functional/no-expression-statements | ||
stepIndex++; | ||
} |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Markdoc AST nodes for custom tags typically have type 'tag' with a 'tag' (or 'name') property indicating the tag name; comparing child.type === 'Step' will never match, so the index is never assigned. Replace the condition with something like if (child.tag === 'step') (aligned with the lowercase schema key).
Copilot uses AI. Check for mistakes.
if (child.type === 'Step') { | ||
// Mutate the child node to add the index to its attributes. | ||
// eslint-disable-next-line functional/immutable-data,functional/no-expression-statements | ||
child.attributes.index = stepIndex; |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You assign an 'index' attribute, but the React Step component consumes 'stepNumber' (added later via cloning in Stepper) and never uses 'index'; this creates dead, confusing data. Either rename to 'stepNumber' and consume it to avoid double work, or remove the mutation entirely.
child.attributes.index = stepIndex; | |
// Removed dead assignment: child.attributes.index = stepIndex; |
Copilot uses AI. Check for mistakes.
import { StepperProps } from 'gitbook-docs/markdoc/schema/stepper'; | ||
|
||
const Stepper = ({ children }: StepperProps<ReactNode>) => ( | ||
<Box sx={{ gap: 0 }}> |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sx prop sets gap: 0 but Box is not given display: 'flex' or 'grid'; gap has no effect on a block div. Either remove gap or add display: 'flex' (or appropriate layout) if spacing was intended.
<Box sx={{ gap: 0 }}> | |
<Box> |
Copilot uses AI. Check for mistakes.
Co-authored-by: marcobottaro <[email protected]>
…mponents/Step.tsx Co-authored-by: marcobottaro <[email protected]>
Jira Pull Request LinkThis Pull Request refers to the following Jira issue DEV-2984 |
Branch is not up to date with base branch@MarBert it seems this Pull Request is not updated with base branch. |
List of Changes
Adds rendering for gitbook steps
Motivation and Context
How Has This Been Tested?
on localhost
Screenshots (if appropriate):
Types of changes
Checklist: