-
Notifications
You must be signed in to change notification settings - Fork 376
chore(grid,stack): update examples flags from js to ts #12199
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
WalkthroughThis PR refactors Grid and Stack layout examples by moving inline example code from markdown documentation files into separate TypeScript/TSX example files. Six new Grid example components and two Stack example components are added, with markdown files updated to reference these external files instead of embedding code inline. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 6
🤖 Fix all issues with AI Agents
In @packages/react-core/src/layouts/Grid/examples/Grid.md:
- Around line 42-44: Remove the extra blank line immediately following the
closing triple-backtick of the GridGroupingOrdering.tsx code fence in Grid.md so
the closing ``` is directly followed by the next section; locate the code fence
that wraps GridGroupingOrdering.tsx and delete the single blank line between the
fence and the subsequent content to match the other examples.
In @packages/react-core/src/layouts/Grid/examples/GridBasic.tsx:
- Around line 1-3: The component GridBasic uses the React type
React.FunctionComponent but React is not imported; fix by importing React (e.g.,
add "import React from 'react'") or import the type directly (e.g., "import type
{ FunctionComponent } from 'react'" and use FunctionComponent for GridBasic) so
the React type annotation resolves.
In @packages/react-core/src/layouts/Grid/examples/GridGroupingOrdering.tsx:
- Around line 1-3: The file declares the component as React.FunctionComponent
but never imports React, causing a TypeScript error; fix by importing the React
types (e.g., add an import for React or import { FunctionComponent } from
'react') and update the component signature if you choose the named import;
ensure the symbol React.FunctionComponent or FunctionComponent is available
before using GridGroupingOrdering's type annotation.
In @packages/react-core/src/layouts/Grid/examples/GridWithOverrides.tsx:
- Line 6: The text content in the GridWithOverrides example has inconsistent
spacing around assignment-like tokens: "span = 3" vs "rowSpan= 2"; update the
example in GridWithOverrides (where "span" and "rowSpan" appear) to use
consistent spacing around equals (e.g., "span = 3" and "rowSpan = 2") so both
tokens match the repo's spacing style.
- Around line 1-3: The component GridWithOverrides uses the
React.FunctionComponent type but React is not imported, causing a TS error; fix
by adding an import for React (e.g., import React or import * as React from
'react') at the top of the file or alternatively change the annotation to a
locally imported type like FC from 'react' and import that (so update the file
to import React or FC and keep the component signature GridWithOverrides:
React.FunctionComponent or GridWithOverrides: FC accordingly).
In @packages/react-core/src/layouts/Stack/examples/StackWithGutter.tsx:
- Around line 1-3: The component StackWithGutter uses the type
React.FunctionComponent but React is not imported; fix by adding the React
import or switching to an imported alias (e.g., import React from 'react' or
import React, { FC } from 'react' and update the declaration to use FC) so the
type annotation for StackWithGutter (React.FunctionComponent) resolves
correctly.
🧹 Nitpick comments (1)
packages/react-core/src/layouts/Stack/examples/StackWithGutter.tsx (1)
6-6: Consider more descriptive example content.The text "pf-m-fill" appears to be a CSS class name rather than typical example content. While it may intentionally indicate which item has the fill behavior, more user-friendly text could improve clarity for developers learning the component.
🔎 Suggested alternative
- <StackItem isFilled>pf-m-fill</StackItem> + <StackItem isFilled>filled content</StackItem>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/react-core/src/layouts/Grid/examples/Grid.mdpackages/react-core/src/layouts/Grid/examples/GridAlternativeComponents.tsxpackages/react-core/src/layouts/Grid/examples/GridBasic.tsxpackages/react-core/src/layouts/Grid/examples/GridGroupingOrdering.tsxpackages/react-core/src/layouts/Grid/examples/GridResponsiveOrdering.tsxpackages/react-core/src/layouts/Grid/examples/GridStandardOrdering.tsxpackages/react-core/src/layouts/Grid/examples/GridWithGutters.tsxpackages/react-core/src/layouts/Grid/examples/GridWithOverrides.tsxpackages/react-core/src/layouts/Stack/examples/Stack.mdpackages/react-core/src/layouts/Stack/examples/StackBasic.tsxpackages/react-core/src/layouts/Stack/examples/StackWithGutter.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/react-core/src/layouts/Stack/examples/StackWithGutter.tsx (2)
packages/react-core/src/layouts/Stack/Stack.tsx (1)
Stack(15-28)packages/react-core/src/layouts/Stack/StackItem.tsx (1)
StackItem(15-28)
packages/react-core/src/layouts/Stack/examples/StackBasic.tsx (2)
packages/react-core/src/layouts/Stack/Stack.tsx (1)
Stack(15-28)packages/react-core/src/layouts/Stack/StackItem.tsx (1)
StackItem(15-28)
⏰ 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). (2)
- GitHub Check: Build
- GitHub Check: Build, test & deploy
🔇 Additional comments (7)
packages/react-core/src/layouts/Stack/examples/StackBasic.tsx (1)
1-9: LGTM! Clean Stack example migration.The example correctly demonstrates basic Stack usage with
isFilledon the middle item. The structure is clear and follows React best practices with the modern JSX transform.packages/react-core/src/layouts/Grid/examples/GridAlternativeComponents.tsx (1)
1-11: LGTM! Good example of semantic HTML with Grid.The example correctly demonstrates using alternative HTML elements (
ul/li) with Grid components, which is useful for maintaining semantic HTML structure.packages/react-core/src/layouts/Grid/examples/GridWithGutters.tsx (1)
1-22: LGTM! Comprehensive Grid layout example.The example effectively demonstrates Grid with gutters and various
spanandrowSpanconfigurations. The numeric values for span properties are correct and the layout clearly shows the Grid's capabilities.packages/react-core/src/layouts/Grid/examples/GridResponsiveOrdering.tsx (1)
5-7: Same order prop type concern applies here.This example also uses string values for the
orderprop ('2','-1','1'). The responsive breakpoint syntax looks correct, but the same verification is needed as in GridStandardOrdering.tsx to confirm whether order values should be strings or numbers.packages/react-core/src/layouts/Grid/examples/GridStandardOrdering.tsx (1)
5-7: No issue found. Theorderprop in GridItem is intentionally designed to accept string values as shown in its type definition (default?: string; md?: string; lg?: string; xl?: string; '2xl'?: string;). The example code correctly uses strings ('2', '-1') which matches the component's prop type.packages/react-core/src/layouts/Grid/examples/Grid.md (1)
15-48: LGTM: Refactoring to external TypeScript examples.The migration from inline JavaScript code blocks to external TypeScript file references successfully achieves the PR objective. The consistent use of
tslanguage identifier andfileattribute properly indicates these are TypeScript examples.packages/react-core/src/layouts/Stack/examples/Stack.md (1)
15-21: LGTM: Refactoring to external TypeScript examples.The migration from inline code blocks to external TypeScript file references is consistent with the Grid examples and successfully updates the example flags from js to ts as intended.
| ```ts file="GridGroupingOrdering.tsx" | ||
|
|
||
| ``` |
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.
Extra blank line after code fence.
There's an unnecessary blank line (line 43) between the code fence closing and the next section. This is inconsistent with the other examples in the file.
🔎 Proposed fix
```ts file="GridGroupingOrdering.tsx"
-
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🤖 Prompt for AI Agents
In @packages/react-core/src/layouts/Grid/examples/Grid.md around lines 42 - 44,
Remove the extra blank line immediately following the closing triple-backtick of
the GridGroupingOrdering.tsx code fence in Grid.md so the closing ``` is
directly followed by the next section; locate the code fence that wraps
GridGroupingOrdering.tsx and delete the single blank line between the fence and
the subsequent content to match the other examples.
| export const GridWithOverrides: React.FunctionComponent = () => ( | ||
| <Grid sm={6} md={4} lg={3} xl2={1}> | ||
| <GridItem span={3} rowSpan={2}> | ||
| span = 3 rowSpan= 2 |
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.
Inconsistent spacing in text content.
The spacing around the equals sign is inconsistent: "span = 3" uses spaces while "rowSpan= 2" is missing a space before the equals sign.
🔎 Proposed fix
- span = 3 rowSpan= 2
+ span = 3 rowSpan = 2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| span = 3 rowSpan= 2 | |
| span = 3 rowSpan = 2 |
🤖 Prompt for AI Agents
In @packages/react-core/src/layouts/Grid/examples/GridWithOverrides.tsx at line
6, The text content in the GridWithOverrides example has inconsistent spacing
around assignment-like tokens: "span = 3" vs "rowSpan= 2"; update the example in
GridWithOverrides (where "span" and "rowSpan" appear) to use consistent spacing
around equals (e.g., "span = 3" and "rowSpan = 2") so both tokens match the
repo's spacing style.
|
Preview: https://pf-react-pr-12199.surge.sh A11y report: https://pf-react-pr-12199-a11y.surge.sh |
Towards #11952
Summary by CodeRabbit
Release Notes
Documentation
New Features
✏️ Tip: You can customize this high-level summary in your review settings.