Push ynlmnrwpxkql #48
Conversation
✅ Deploy Preview for determinate-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
grahamc
force-pushed
the
push-ynlmnrwpxkql
branch
from
161fa56 to
32bbd3f
Compare
grahamc
force-pushed
the
push-ynlmnrwpxkql
branch
5 times, most recently
from
0be93a7 to
96c7434
Compare
grahamc
force-pushed
the
push-ynlmnrwpxkql
branch
from
96c7434 to
250f7c7
Compare
WalkthroughAdds CodeBlock file-mode and download support, removes the CodeFile component and stories, overhauls InstallCTA into tab-driven, version-aware flows (adds AWS/GitHub/NixOS content), introduces a Navigation component and stories, adds Sass mixins/tokens, makes SSR-safe color handling, adds Nix highlighting, and adjusts various styles and exports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CTA as InstallCTA
participant TS as TabSelector
participant Map as ctaComponents
participant TB as TabBody
participant CB as CodeBlock
U->>CTA: Load (initialTab, version?)
CTA->>TS: render tabs from ctaTabs
U->>TS: select tab
TS-->>CTA: activeTab
CTA->>Map: resolve active component
Map-->>CTA: TabBody component
CTA->>TB: render <TabBody version={version}/>
TB->>CB: render code (kind, allowCopy, allowDownload)
alt kind == "file" and allowDownload
CB->>CB: show DownloadButton (filename from downloadAs/title)
end
alt allowCopy
CB->>CB: show CopyButton (data = trimmedCode)
end
sequenceDiagram
autonumber
participant P as Props
participant CB as CodeBlock
participant HL as Highlight
P-->>CB: {code, language, title, kind, allowCopy, allowDownload, downloadAs?}
CB->>CB: trimmedCode = code.trim()
CB->>HL: Highlight(language, trimmedCode)
CB->>CB: Conditionally render Copy/Download icons based on props
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
Comment |
grahamc
force-pushed
the
push-ynlmnrwpxkql
branch
from
250f7c7 to
c07e350
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (26)
package.json (1)
51-53: Peer dep set to React 19 only—confirm intent; consider allowing React 18 too.If you don’t use React 19–specific APIs, widening helps consumers still on 18.
Apply this diff if you want to support 18 and 19:
- "react": "^19" + "react": ">=18 <20"src/InstallCTA/TabSelector.tsx (2)
4-8: Type the icon as ComponentType rather than FC for better compatibility with react-icons.react-icons exports
IconType = ComponentType<IconBaseProps>;FCis unnecessarily restrictive.- icon: FC<IconBaseProps>; + icon: ComponentType<IconBaseProps>;Add the type import (outside this range):
import { useCallback, type FC, type MouseEvent, type ComponentType } from "react";
38-38: Avoid duplicate accessible names; make the icon decorative.The link already contains text (
{name}), so the icon should be aria-hidden instead of getting a title.- <Icon title={name} /> {name} + <Icon aria-hidden="true" focusable="false" /> {name}src/hooks/useSystemColorScheme.ts (1)
35-52: Add Safari <14 fallback for media query change events.Older Safari only supports
addListener/removeListener.- media.addEventListener("change", handleChange); - return function cleanup() { - media.removeEventListener("change", handleChange); - }; + if ("addEventListener" in media) { + media.addEventListener("change", handleChange); + return () => media.removeEventListener("change", handleChange); + } else { + // Safari <14 + // @ts-expect-error legacy API + media.addListener(handleChange); + return () => { + // @ts-expect-error legacy API + media.removeListener(handleChange); + }; + }src/ColorProvider/index.tsx (4)
97-105: Don’t reassign the destructured prop; compute a resolvedRoot instead.Improves clarity and avoids param shadowing.
- root, + root, children, }) => { - if (!root) { - if (typeof document !== "undefined") { - root = document.body; - } - } + const resolvedRoot = + root ?? (typeof document !== "undefined" ? document.body : undefined);
118-121: Avoid DOM side effects during render; use a layout effect for initial apply.This prevents render-time mutations and still avoids FOUC.
- if (root) { - applyTheme(root, scheme); - } + // Apply ASAP before paint to reduce FOUC + // (import useLayoutEffect at top) + useLayoutEffect(() => { + if (resolvedRoot) { + applyTheme(resolvedRoot, scheme); + } + }, []);Add import (outside this range):
import React, { useEffect, useLayoutEffect, useMemo, useState, type PropsWithChildren } from "react";
138-141: Use the resolved root variable consistently.Keeps semantics consistent with the earlier resolution.
- if (root) { - applyTheme(root, scheme); + if (resolvedRoot) { + applyTheme(resolvedRoot, scheme); }
150-154: Avoid console noise in libraries.Drop or gate the log in production.
- if (classes === undefined) { - console.log("classList on the root is null... probably SSR?"); - return; - } + if (classes === undefined) { + return; + }src/sass/_tokens.scss (1)
104-106: Spacing scale:lgduplicatesbase.
baseandlgare both0.75rem, which defeats the scale. Recommend monotonic increase.Example adjustment:
$spacing: ( xs: 0.25rem, sm: 0.5rem, base: 0.75rem, - lg: 0.75rem, - xl: 1rem, + lg: 1rem, + xl: 1.25rem, );src/sass/_mixins.scss (2)
49-51: Guard radius math against edge cases.If a future radius token drops below
1px,calc(... - 1px)can go negative. Cheap guard withmax().- border-top-left-radius: calc(map.get(tokens.$border-radius, $size) - 1px); - border-top-right-radius: calc(map.get(tokens.$border-radius, $size) - 1px); + border-top-left-radius: max(calc(map.get(tokens.$border-radius, $size) - 1px), 0px); + border-top-right-radius: max(calc(map.get(tokens.$border-radius, $size) - 1px), 0px); @@ - border-bottom-left-radius: calc(map.get(tokens.$border-radius, $size) - 1px); - border-bottom-right-radius: calc(map.get(tokens.$border-radius, $size) - 1px); + border-bottom-left-radius: max(calc(map.get(tokens.$border-radius, $size) - 1px), 0px); + border-bottom-right-radius: max(calc(map.get(tokens.$border-radius, $size) - 1px), 0px);Also applies to: 69-71
82-95: DRY nit: duplicate box-shadow in light mode.Same shadow in both modes—drop the override unless you plan to diverge later.
@mixin shadow { box-shadow: 0 0 #0000, 0 0 #0000, 0 1px 3px 0 rgba(0, 0, 0, 0.1), 0 1px 2px -1px rgba(0, 0, 0, 0.1); - @include light-mode { - box-shadow: - 0 0 #0000, - 0 0 #0000, - 0 1px 3px 0 rgba(0, 0, 0, 0.1), - 0 1px 2px -1px rgba(0, 0, 0, 0.1); - } }src/CodeBlock/CodeBlock.stories.tsx (1)
14-17: Stories read well; consider adding a Nix example and a filename for downloads.
- Add a small Nix snippet to showcase the new language.
- Provide
downloadAs(e.g., ".github/workflows/nix-ci.yaml") so downloads have a friendly filename.Example:
export const GithubCI: Story = { args: { @@ - allowDownload: true, + allowDownload: true, + downloadAs: ".github/workflows/nix-ci.yaml", }, }; @@ +export const NixSample: Story = { + args: { + language: "nix", + code: 'pkgs: pkgs.mkShell { buildInputs = [ pkgs.hello ]; }', + title: "shell.nix", + kind: "file", + allowCopy: true, + }, +};Also applies to: 21-23, 65-93, 95-115
src/InstallCTA/FindAwsAMIs.tsx (2)
12-14: Type the component as FC and drop the unused paramPrevents mismatched prop typing and removes the placeholder arg.
-const FindAwsAMIs: FC = (_: TabProps) => { +const FindAwsAMIs: FC<TabProps> = () => {
6-10: Tighten the AMI search to only available imagesAdds a safe state filter; keeps output deterministic.
const code = `aws ec2 describe-images \ --owners 535002876703 \ --filters "Name=name,Values=determinate/nixos/epoch-1/*" \ - "Name=architecture,Values=x86_64,arm64" \ + "Name=architecture,Values=x86_64,arm64" \ + "Name=state,Values=available" \ --query "Images | sort_by(@, &CreationDate) | [-1]"`;src/InstallCTA/InstallForNixOS.tsx (1)
16-19: Type the component as FCAvoids accepting {} props at the type level.
-const InstallForNixOS: FC = ({ version }: TabProps) => { +const InstallForNixOS: FC<TabProps> = ({ version }) => {src/InstallCTA/index.tsx (1)
71-89: A11y: ensure proper tab semantics in TabSelectorTab lists should expose role="tablist"/"tab" and aria-selected; verify TabSelector does this.
src/CodeBlock/index.tsx (1)
47-58: Prevent rendering an empty icon wrapperOnly show the icons container when at least one control will render.
- {(allowCopy || allowDownload) && ( + {(allowCopy || (allowDownload && isFile)) && ( <span className="code-block__icons"> {allowCopy && <CopyButton data={trimmedCode} />} {allowDownload && isFile && ( <DownloadButton data={trimmedCode} filename={downloadAs.split("/").reverse()[0]} /> )} </span> )}src/InstallCTA/UseWithGitHubActions.tsx (2)
6-7: Type the component as FCTightens prop typing.
-const UseWithGitHubActions: FC = ({ version }: TabProps) => { +const UseWithGitHubActions: FC<TabProps> = ({ version }) => {
7-18: Option: set minimal permissions in the workflowSensible default for least privilege.
const code = ` on: push: +permissions: + contents: read jobs: tests: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v5 - uses: DeterminateSystems/determinate-nix-action@v${version || "3"} - run: nix build .# `;src/CodeBlock/index.scss (1)
11-15: Naming consistency: prefer a BEM-scoped class over .highlightMinor: consider .code-block__highlight to avoid coupling to global .highlight naming.
src/InstallCTA/InstallCTA.stories.tsx (6)
9-23: Use Storybook controls instead of rawtypein argTypes
type: "string"isn’t a control and may be ignored by Storybook. Recommend proper controls and (optionally) deriving options from the enum for future-proofing.argTypes: { initialTab: { - type: "string", - options: [ + control: { type: "select" }, + options: [ InstallTarget.AWS, InstallTarget.GitHub, InstallTarget.Linux, InstallTarget.MacOS, InstallTarget.NixOS, ], }, version: { - type: "string", + control: { type: "text" }, }, },
33-35: Fix inconsistent story nameMacOSLatest is labeled “macOS Tagged”. Rename to “macOS Latest” to match intent and avoid confusion with MacOSTagged.
export const MacOSLatest: Story = { - name: "macOS Tagged", + name: "macOS Latest",
40-44: Add name for consistencyOther “Latest” stories have readable names; add one here for uniformity.
export const LinuxLatest: Story = { + name: "Linux Latest", args: { initialTab: InstallTarget.Linux, }, };
46-50: Add name for consistencyMatch labeling with GitHub/NixOS Latest.
export const AWSLatest: Story = { + name: "AWS Latest", args: { initialTab: InstallTarget.AWS, }, };
74-79: Add name for consistencyProvide a friendly label like other Tagged stories.
export const LinuxTagged: Story = { + name: "Linux Tagged", args: { initialTab: InstallTarget.Linux, version: "3.8.6", }, };
81-86: Add name for consistencyAlign with GitHubTagged/NixOSTagged.
export const AWSTagged: Story = { + name: "AWS Tagged", args: { initialTab: InstallTarget.AWS, version: "3.8.6", }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
package.json(1 hunks)src/CodeBlock/CodeBlock.stories.tsx(2 hunks)src/CodeBlock/index.scss(1 hunks)src/CodeBlock/index.tsx(2 hunks)src/CodeFile/CodeFile.stories.tsx(0 hunks)src/CodeFile/index.scss(0 hunks)src/CodeFile/index.tsx(0 hunks)src/ColorProvider/index.tsx(4 hunks)src/Highlight/index.scss(1 hunks)src/InstallCTA/FindAwsAMIs.tsx(1 hunks)src/InstallCTA/InstallCTA.stories.tsx(2 hunks)src/InstallCTA/InstallForNixOS.tsx(1 hunks)src/InstallCTA/InstallFromCurl.tsx(1 hunks)src/InstallCTA/NavTab.tsx(0 hunks)src/InstallCTA/TabSelector.tsx(2 hunks)src/InstallCTA/UseWithGitHubActions.tsx(1 hunks)src/InstallCTA/index.scss(2 hunks)src/InstallCTA/index.tsx(3 hunks)src/InstallCTA/types.ts(2 hunks)src/Link/index.scss(1 hunks)src/MacInstaller/index.scss(2 hunks)src/MacInstaller/index.tsx(1 hunks)src/hooks/useHighlight.ts(3 hunks)src/hooks/useSystemColorScheme.ts(2 hunks)src/index.ts(0 hunks)src/sass/_mixins.scss(1 hunks)src/sass/_tokens.scss(2 hunks)
💤 Files with no reviewable changes (5)
- src/CodeFile/index.scss
- src/CodeFile/index.tsx
- src/CodeFile/CodeFile.stories.tsx
- src/InstallCTA/NavTab.tsx
- src/index.ts
🧰 Additional context used
🧬 Code graph analysis (7)
src/InstallCTA/InstallForNixOS.tsx (1)
src/InstallCTA/index.tsx (1)
TabProps(32-37)
src/InstallCTA/types.ts (1)
src/InstallCTA/index.tsx (1)
InstallTarget(17-17)
src/CodeBlock/index.tsx (2)
src/index.ts (2)
CodeBlockProps(15-15)HighlightLanguage(7-7)src/hooks/useHighlight.ts (1)
HighlightLanguage(21-21)
src/InstallCTA/FindAwsAMIs.tsx (1)
src/InstallCTA/index.tsx (1)
TabProps(32-37)
src/InstallCTA/InstallFromCurl.tsx (1)
src/InstallCTA/index.tsx (1)
TabProps(32-37)
src/InstallCTA/UseWithGitHubActions.tsx (1)
src/InstallCTA/index.tsx (1)
TabProps(32-37)
src/InstallCTA/InstallCTA.stories.tsx (1)
src/InstallCTA/index.tsx (1)
InstallTarget(17-17)
🪛 Biome (2.1.2)
src/InstallCTA/index.tsx
[error] 78-83: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (14)
src/InstallCTA/types.ts (2)
8-10: LGTM: targets expanded (AWS, GitHub).Enum additions look consistent with the rest of the PR. Ensure stories and the tab/component map cover these targets.
17-22: SSR safety: avoid defaulting to navigator in the parameter; confirm Windows→Linux mapping.Defaulting userAgent to navigator.userAgent will throw during SSR — make the param optional and read navigator at runtime if available. Also confirm the product decision to map Windows → InstallTarget.Linux.
File: src/InstallCTA/types.ts Lines: 17-22 (also applies to 24-28)
-export function detectInstallTarget( - userAgent = navigator.userAgent, -): InstallTarget { - if (userAgent.includes("Windows")) { +export function detectInstallTarget(userAgent?: string): InstallTarget { + const ua = + userAgent ?? (typeof navigator !== "undefined" ? navigator.userAgent : ""); + if (ua.includes("Windows")) { return InstallTarget.Linux; } - if (userAgent.includes("Linux")) { + if (ua.includes("Linux")) { return InstallTarget.Linux; } return InstallTarget.MacOS; }src/hooks/useSystemColorScheme.ts (1)
5-10: Nice SSR guard on window access.Prevents crashes during SSR while preserving client behavior.
src/Link/index.scss (1)
8-9: Good a11y improvement.Underline + offset improves discoverability beyond color alone.
src/sass/_tokens.scss (1)
44-46: Code bg color change acknowledged.Switching
bg-darkto#231f20aligns with brand black; verify contrast with Shiki dark token colors in real code samples.src/Highlight/index.scss (1)
11-11: Removing CSS var fallbacks is fine with our Shiki config.Given
defaultColor: false, relying solely on token-driven backgrounds is consistent.Also applies to: 14-15
src/sass/_mixins.scss (1)
31-40: Solid base border mixin.Consistent radius and color with light-mode override reads well.
src/InstallCTA/InstallForNixOS.tsx (1)
7-14: Use inputs.determinate for module reference to match the input declarationThe snippet declares inputs.determinate; reference it consistently.
- # Load the Determinate module - determinate.nixosModules.default + # Load the Determinate module + inputs.determinate.nixosModules.defaultIf your docs intentionally show the parameter-style binding (outputs = { self, nixpkgs, determinate, ... }:), ignore this change. Otherwise, this keeps the two snippets consistent.
src/MacInstaller/index.scss (1)
15-26: LGTMNice light-mode theming and spacing tweaks.
Also applies to: 29-32, 38-50, 56-57
src/InstallCTA/InstallCTA.stories.tsx (5)
52-57: LGTMStory name and args look good.
59-64: LGTMConsistent with other “Latest” stories.
66-72: LGTMTagged story and version wiring look good.
88-94: LGTMNaming and args are consistent.
96-102: LGTMLooks consistent with the rest.
| kind: "snippet" | "file"; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make kind optional for backward compatibility
You already default kind to "snippet"; let callers omit it.
- kind: "snippet" | "file";
+ kind?: "snippet" | "file";Also ensure the exported type mirrors this change.
Also applies to: 24-33
🏁 Script executed:
#!/bin/bash
# Verify external exports and callsites for kind
rg -nP 'export\s+(type|interface)\s+CodeBlockProps' src/index.ts
rg -nP --type=tsx '<CodeBlock(?![^>]*\bkind=)' -C1Length of output: 83
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "=== List src/CodeBlock ==="
ls -la src/CodeBlock 2>/dev/null || echo "src/CodeBlock does not exist"
echo
echo "=== Show src/CodeBlock/index.tsx (first 240 lines) ==="
if [[ -f src/CodeBlock/index.tsx ]]; then
sed -n '1,240p' src/CodeBlock/index.tsx | nl -ba -v1
else
echo "MISSING: src/CodeBlock/index.tsx"
fi
echo
echo "=== Search for CodeBlockProps occurrences ==="
rg -n --hidden -S "CodeBlockProps" src || true
echo
echo "=== Search for 'export type|interface CodeBlockProps' ==="
rg -n --hidden -S "export\s+(type|interface)\s+CodeBlockProps" src || true
echo
echo "=== Search for 'kind:' definitions ==="
rg -n --hidden -S "kind\s*:" src || true
echo
echo "=== Find files with <CodeBlock and show context (3 lines) ==="
rg -n --hidden -S "<CodeBlock" -g 'src/**' -C3 || true
echo
echo "=== For each file containing <CodeBlock, check if file contains 'kind=' anywhere ==="
files=$(rg -n --hidden -S "<CodeBlock" -g 'src/**' --files || true)
if [[ -z "${files// /}" ]]; then
echo "No files with <CodeBlock found under src/"
else
for f in $files; do
echo "---- File: $f ----"
rg -n --hidden -S "<CodeBlock" -n -C3 "$f" || true
echo "---- 'kind=' occurrences in $f ----"
rg -n --hidden -S "kind\s*=" -n -C3 "$f" || echo "no kind= in $f"
done
fiLength of output: 455
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "=== Show src/CodeBlock/index.tsx ==="
if [[ -f src/CodeBlock/index.tsx ]]; then
sed -n '1,400p' src/CodeBlock/index.tsx || true
else
echo "MISSING: src/CodeBlock/index.tsx"
fi
echo
echo "=== Search for CodeBlockProps occurrences ==="
rg -n --hidden --no-ignore "CodeBlockProps" src || true
echo
echo "=== Search for explicit exports of CodeBlockProps ==="
rg -n --hidden --no-ignore "export type CodeBlockProps" src || true
rg -n --hidden --no-ignore "export interface CodeBlockProps" src || true
echo
echo "=== Search for 'kind' property definitions/usages ==="
rg -n --hidden --no-ignore "kind\s*:" src || true
rg -n --hidden --no-ignore "\"snippet\"\s*\|\s*\"file\"" src || true
echo
echo "=== Find <CodeBlock components and whether they pass kind= ==="
rg -n --hidden --no-ignore "<CodeBlock" -g 'src/**' -C2 || true
files=$(rg --hidden --no-ignore "<CodeBlock" -g 'src/**' --files || true)
if [[ -z "${files// /}" ]]; then
echo "No files with <CodeBlock found under src/"
else
for f in $files; do
echo
echo "---- File: $f ----"
sed -n '1,200p' "$f" || true
echo "---- 'kind=' occurrences in $f ----"
rg -n --hidden --no-ignore "kind\s*=" -n -C2 "$f" || echo "no kind= in $f"
done
fiLength of output: 5026
Make CodeBlockProps.kind optional
Align the exported type with the component default and existing call sites.
- kind: "snippet" | "file";
+ kind?: "snippet" | "file";Edit: src/CodeBlock/index.tsx (interface CodeBlockProps — kind at ~line 18). src/index.ts re-exports the type — no change needed.
📝 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.
| kind: "snippet" | "file"; | |
| } | |
| kind?: "snippet" | "file"; | |
| } |
🤖 Prompt for AI Agents
In src/CodeBlock/index.tsx around lines 18-19, the CodeBlockProps.interface
marks kind as required but the component and existing callers treat it as
optional; change the interface to make kind optional (kind?: "snippet" |
"file"), leaving any re-exports unchanged, and run typecheck to ensure no other
call sites need updates.
| import langShell from "@shikijs/langs/shellscript"; | ||
| import langTerraform from "@shikijs/langs/terraform"; | ||
| import langYaml from "@shikijs/langs/yaml"; | ||
| import langNix from "@shikijs/langs/nix"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Nix support looks good; add a safe no-highlight path for "text".
If "text" isn’t a registered Shiki language, codeToHtml can throw. Short-circuit when language === "text" to return escaped HTML without invoking Shiki. Also consider catching unknown langs to avoid runtime crashes.
Apply inside useHighlight:
function useHighlight(language: HighlightLanguage, code: string): string {
return useMemo(() => {
+ if (language === "text") {
+ const escape = (s: string) =>
+ s.replace(/[&<>"']/g, (ch) => ({ "&":"&","<":"<",">":">",'"':""","'":"'" }[ch]!));
+ return `<pre class="highlight"><code>${escape(code)}</code></pre>`;
+ }
const html = getShiki().codeToHtml(code, {
lang: language,
themes: {
light: "github-light",
dark: "github-dark",
},
defaultColor: false,
structure: "inline",
});
return html;
}, [language, code]);
}Also applies to: 15-20, 28-29
🤖 Prompt for AI Agents
In src/hooks/useHighlight.ts around lines 6, 15-20 and 28-29, add a safe
no-highlight path for language === "text" and guard against unknown Shiki
languages: short-circuit before calling shiki.codeToHtml to return HTML-escaped
code for plain text, and wrap the shiki highlighting call in a try/catch (or
check shiki.getLanguage) to fall back to escaped HTML when the requested
language isn't registered or an error occurs; ensure you import or use an
existing HTML-escape utility and return the same shape as the successful
codeToHtml result.
| display: flex; | ||
| text-align: center; | ||
| align-items: center; | ||
| text-wrap-mode: nowrap; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use white-space: nowrap (text-wrap-mode is nonstandard/unsupported)
Fixes a CSS correctness issue.
- text-wrap-mode: nowrap;
+ white-space: nowrap;📝 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.
| display: flex; | |
| text-align: center; | |
| align-items: center; | |
| text-wrap-mode: nowrap; | |
| display: flex; | |
| text-align: center; | |
| align-items: center; | |
| white-space: nowrap; |
🤖 Prompt for AI Agents
In src/InstallCTA/index.scss around lines 59 to 63, the stylesheet uses the
nonstandard property text-wrap-mode: nowrap; — replace that line with the
standard property white-space: nowrap; so the text will not wrap across
browsers; keep the surrounding declarations (display, text-align, align-items)
as-is.
| {ctaTabs.map(([target, icon]) => ( | ||
| <TabSelector | ||
| name={target} | ||
| icon={icon} | ||
| active={activeTab == target} | ||
| onClick={() => setActiveTab(target)} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a key to mapped elements; fix strict equality
Resolves the Biome error and avoids accidental coercion.
- {ctaTabs.map(([target, icon]) => (
- <TabSelector
+ {ctaTabs.map(([target, icon]) => (
+ <TabSelector
+ key={target}
name={target}
icon={icon}
- active={activeTab == target}
+ active={activeTab === target}
onClick={() => setActiveTab(target)}
/>
))}📝 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.
| {ctaTabs.map(([target, icon]) => ( | |
| <TabSelector | |
| name={target} | |
| icon={icon} | |
| active={activeTab == target} | |
| onClick={() => setActiveTab(target)} | |
| /> | |
| ))} | |
| {ctaTabs.map(([target, icon]) => ( | |
| <TabSelector | |
| key={target} | |
| name={target} | |
| icon={icon} | |
| active={activeTab === target} | |
| onClick={() => setActiveTab(target)} | |
| /> | |
| ))} |
🧰 Tools
🪛 Biome (2.1.2)
[error] 78-83: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🤖 Prompt for AI Agents
In src/InstallCTA/index.tsx around lines 77 to 84, the mapped TabSelector
elements are missing a key prop and use loose equality; add a unique key (e.g.,
key={target}) to the TabSelector returned by the map and change the active
comparison from == to strict === (activeTab === target) to satisfy React/Biome
rules and avoid accidental coercion.
| const installScript = (version: string | undefined) => { | ||
| if (typeof version === "undefined") { | ||
| return "curl -fsSL https://install.determinate.systems/nix | sh -s -- install --determinate"; | ||
| } else { | ||
| return `curl -fsSL https://install.determinate.systems/nix/tag/v${version} | sh -s -- install --determinate`; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sanitize version before embedding in a shell command URL.
Unencoded versions can break the URL users copy-paste; encode for safety.
-const installScript = (version: string | undefined) => {
- if (typeof version === "undefined") {
+const installScript = (version: string | undefined) => {
+ if (typeof version === "undefined" || version === "") {
return "curl -fsSL https://install.determinate.systems/nix | sh -s -- install --determinate";
} else {
- return `curl -fsSL https://install.determinate.systems/nix/tag/v${version} | sh -s -- install --determinate`;
+ const v = encodeURIComponent(version);
+ return `curl -fsSL https://install.determinate.systems/nix/tag/v${v} | sh -s -- install --determinate`;
}
};📝 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.
| const installScript = (version: string | undefined) => { | |
| if (typeof version === "undefined") { | |
| return "curl -fsSL https://install.determinate.systems/nix | sh -s -- install --determinate"; | |
| } else { | |
| return `curl -fsSL https://install.determinate.systems/nix/tag/v${version} | sh -s -- install --determinate`; | |
| } | |
| }; | |
| const installScript = (version: string | undefined) => { | |
| if (typeof version === "undefined" || version === "") { | |
| return "curl -fsSL https://install.determinate.systems/nix | sh -s -- install --determinate"; | |
| } else { | |
| const v = encodeURIComponent(version); | |
| return `curl -fsSL https://install.determinate.systems/nix/tag/v${v} | sh -s -- install --determinate`; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/InstallCTA/InstallFromCurl.tsx around lines 6 to 12, the version string
is interpolated directly into the install URL which can break or introduce
unsafe characters; sanitize the version by URL-encoding it (e.g., use
encodeURIComponent) before embedding into the backtick string, ensure the
undefined branch remains unchanged, and use the encodedVersion variable in the
tag path so the generated curl URL is safe for copy-paste.
| const InstallFromCurl: FC = ({ version }: TabProps) => ( | ||
| <> |
There was a problem hiding this comment.
Props typing bug: component exported as FC<{}> (loses version).
Typing the variable as FC without generics erases the version prop from the public type, causing TS errors for consumers.
Apply:
-const InstallFromCurl: FC = ({ version }: TabProps) => (
+const InstallFromCurl: FC<TabProps> = ({ version }) => (📝 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.
| const InstallFromCurl: FC = ({ version }: TabProps) => ( | |
| <> | |
| const InstallFromCurl: FC<TabProps> = ({ version }) => ( | |
| <> |
🤖 Prompt for AI Agents
In src/InstallCTA/InstallFromCurl.tsx around lines 14 to 15, the component is
typed as FC without generics which erases the version prop; change the component
type to FC<TabProps> and adjust the parameter typing by removing the inline cast
(e.g. keep destructuring { version } without : TabProps) so the exported
component retains the correct public prop type; ensure TabProps is
imported/available and update the signature to use the generic FC<TabProps>.
| const urlPath = version ? `tag/v${version}` : `stable`; | ||
| return ( | ||
| <a | ||
| href={`https://install.determinate.systems/determinate-pkg/${urlPath}/Universal`} | ||
| className="mac-installer" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Encode version in URLs to avoid malformed links.
Protect the path segment when version contains spaces or special chars.
- const urlPath = version ? `tag/v${version}` : `stable`;
+ const urlPath = version ? `tag/v${encodeURIComponent(version)}` : `stable`;📝 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.
| const urlPath = version ? `tag/v${version}` : `stable`; | |
| return ( | |
| <a | |
| href={`https://install.determinate.systems/determinate-pkg/${urlPath}/Universal`} | |
| className="mac-installer" | |
| const urlPath = version ? `tag/v${encodeURIComponent(version)}` : `stable`; | |
| return ( | |
| <a | |
| href={`https://install.determinate.systems/determinate-pkg/${urlPath}/Universal`} | |
| className="mac-installer" |
🤖 Prompt for AI Agents
In src/MacInstaller/index.tsx around lines 17 to 21, the URL uses the raw
version string which can produce malformed links if version contains spaces or
special characters; update the code to URL-encode the version segment (e.g., use
encodeURIComponent on the version value when building urlPath or the href) so
the href becomes safe, preserving the "v" prefix only once and leaving the
stable branch unchanged.
c07e350 to
f0f28a1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/InstallCTA/types.ts (1)
17-19: Make detectInstallTarget SSR-safe (navigator may be undefined).Calling this during SSR will throw. Guard access to navigator and work off a local ua string.
-export function detectInstallTarget( - userAgent = navigator.userAgent, -): InstallTarget { - if (userAgent.includes("Windows")) { +export function detectInstallTarget(userAgent?: string): InstallTarget { + const ua = + userAgent ?? + (typeof navigator !== "undefined" ? navigator.userAgent : ""); + if (ua.includes("Windows")) { return InstallTarget.Linux; } - if (userAgent.includes("Linux")) { + if (ua.includes("Linux")) { return InstallTarget.Linux; } return InstallTarget.MacOS; }Also applies to: 20-29
♻️ Duplicate comments (1)
src/InstallCTA/index.tsx (1)
77-83: Add a key to mapped elements; use strict equality.Avoid React warnings and accidental coercion.
- {ctaTabs.map(([target, icon]) => ( + {ctaTabs.map(([target, icon]) => ( <TabSelector + key={target} name={target} icon={icon} - active={activeTab == target} + active={activeTab === target} onClick={() => setActiveTab(target)} /> ))}
🧹 Nitpick comments (14)
src/InstallCTA/types.ts (1)
24-26: Optionally recognize Android/ChromeOS as Linux.Many Android/CrOS UAs won’t match "Linux" literally.
- if (ua.includes("Linux")) { + if (ua.includes("Linux") || ua.includes("Android") || ua.includes("CrOS")) { return InstallTarget.Linux; }src/InstallCTA/index.tsx (2)
15-15: Prefer IconType over React.FC for react-icons.IconType is the canonical type for these icons. Validate TabSelector’s prop type before changing.
-import type { IconBaseProps } from "react-icons"; +import type { IconType } from "react-icons"; -const ctaTabs: [InstallTarget, React.FC<IconBaseProps>][] = [ +const ctaTabs: [InstallTarget, IconType][] = [Also applies to: 39-45
1-1: Client-only detection to avoid SSR mismatches.Initialize with a stable default and detect on mount. This also sidesteps any future SSR access pitfalls.
-import { useState, type FC } from "react"; +import { useState, useEffect, type FC } from "react"; @@ -const InstallCTA: FC<InstallCTAProps> = ({ initialTab, version }) => { - const [activeTab, setActiveTab] = useState<InstallTarget>(() => { - if (initialTab) { - return initialTab; - } - - return detectInstallTarget(); - }); +const InstallCTA: FC<InstallCTAProps> = ({ initialTab, version }) => { + const [activeTab, setActiveTab] = useState<InstallTarget>( + initialTab ?? InstallTarget.MacOS, + ); + useEffect(() => { + if (!initialTab) { + setActiveTab(detectInstallTarget()); + } + }, [initialTab]);Also applies to: 60-67
src/InstallCTA/InstallCTA.stories.tsx (2)
33-35: Fix mislabeled story name (Latest vs Tagged).MacOSLatest should display “macOS Latest”.
-export const MacOSLatest: Story = { - name: "macOS Tagged", +export const MacOSLatest: Story = { + name: "macOS Latest",
9-23: Enable proper Storybook controls for args.Use control types instead of argTypes.type to get a select/text UI.
argTypes: { initialTab: { - type: "string", + control: { type: "select" }, options: [ InstallTarget.AWS, InstallTarget.GitHub, InstallTarget.Linux, InstallTarget.MacOS, InstallTarget.NixOS, ], }, version: { - type: "string", + control: { type: "text" }, }, },src/PageLayout/index.tsx (1)
26-32: Per-pane wrapper: add a class and avoid index fallback keys.Wrapping each pane is good. Add a specific class for styling hooks and prefer explicit keys over index to avoid reconciliation glitches when panes reorder.
Apply:
- {panes.map((pane, idx) => ( - <div key={pane.key ?? idx}>{pane}</div> - ))} + {panes.map((pane, idx) => ( + <div key={pane.key ?? idx} className="page-layout__pane"> + {pane} + </div> + ))}Please double-check any CSS that assumed panes were direct children of
.page-layout__panes.src/PageLayout/PageLayout.stories.tsx (1)
64-64: Add a key to the single pane element.Eliminates the “key” lint error and avoids index fallback keys.
- panes: [<Placeholder height="3em" label="Pane 1" />], + panes: [<Placeholder key="pane-1" height="3em" label="Pane 1" />],src/Navigation/index.scss (1)
55-61: Avoid fixed widths that can overflow on very narrow viewports.Use a responsive width to prevent horizontal scrolling.
- .navigation__element { - width: 320px; - - @media screen and (width < 650px) { - width: 268px; - } - } + .navigation__element { + /* Fit within viewport while capping at desktop size */ + width: min(320px, 100vw); + }src/Navigation/index.tsx (1)
8-11: Broadenchildrentype and preserve child keys.Support single-child usage and avoid index keys by normalizing via
React.Children.toArrayand using element keys when present.export interface NavigationProps { initialState?: "open" | "closed"; - children: React.ReactNode[]; + children: React.ReactNode; } @@ - {children.map((child, idx) => ( - <div key={idx} className="navigation__element"> - {child} - </div> - ))} + {React.Children.toArray(children).map((child, idx) => { + const key = + React.isValidElement(child) && child.key != null + ? String(child.key) + : String(idx); + return ( + <div key={key} className="navigation__element"> + {child} + </div> + ); + })}Also applies to: 47-51
src/Navigation/Navigation.stories.tsx (5)
27-29: Add keys to children array elements to satisfy React and lint.- children: [ - <Placeholder height="100%" label="Primary Element" />, - <Placeholder height="100%" label="Navigation Link" />, - ], + children: [ + <Placeholder key="primary" height="100%" label="Primary Element" />, + <Placeholder key="link" height="100%" label="Navigation Link" />, + ],
50-52: Add keys in InHeader story.- children: [ - <Placeholder height="100%" label="Lorem" />, - <Placeholder height="100%" label="Ipsum" />, - ], + children: [ + <Placeholder key="lorem" height="100%" label="Lorem" />, + <Placeholder key="ipsum" height="100%" label="Ipsum" />, + ],
69-71: Add keys in DefaultOpen story.- children: [ - <Placeholder height="100%" label="Lorem" />, - <Placeholder height="100%" label="Ipsum" />, - ], + children: [ + <Placeholder key="lorem" height="100%" label="Lorem" />, + <Placeholder key="ipsum" height="100%" label="Ipsum" />, + ],
94-96: Add keys in InHeaderOpen story.- children: [ - <Placeholder height="100%" label="Lorem" />, - <Placeholder height="100%" label="Ipsum" />, - ], + children: [ + <Placeholder key="lorem" height="100%" label="Lorem" />, + <Placeholder key="ipsum" height="100%" label="Ipsum" />, + ],
23-31: Optional: switch stories to JSX children instead of achildrenprop.This aligns with React conventions and eliminates the no-children-prop lint across stories.
Example for Default:
-export const Default: Story = { - args: { - initialState: undefined, - children: [ - <Placeholder key="primary" height="100%" label="Primary Element" />, - <Placeholder key="link" height="100%" label="Navigation Link" />, - ], - }, - render: (props) => ( +export const Default: Story = { + args: { initialState: undefined }, + render: (props) => ( <div style={{ height: "300px", width: "500px", border: "1px solid black", display: "flex", justifyContent: "end" }}> - <Navigation {...props} /> + <Navigation {...props}> + <Placeholder key="primary" height="100%" label="Primary Element" /> + <Placeholder key="link" height="100%" label="Navigation Link" /> + </Navigation> </div> ), };Apply similarly to the other stories.
Also applies to: 46-63, 65-88, 90-107
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
package.json(1 hunks)src/CodeBlock/CodeBlock.stories.tsx(2 hunks)src/CodeBlock/index.scss(1 hunks)src/CodeBlock/index.tsx(2 hunks)src/CodeFile/CodeFile.stories.tsx(0 hunks)src/CodeFile/index.scss(0 hunks)src/CodeFile/index.tsx(0 hunks)src/ColorProvider/index.tsx(4 hunks)src/Header/index.tsx(1 hunks)src/Highlight/index.scss(1 hunks)src/InstallCTA/FindAwsAMIs.tsx(1 hunks)src/InstallCTA/InstallCTA.stories.tsx(2 hunks)src/InstallCTA/InstallForNixOS.tsx(1 hunks)src/InstallCTA/InstallFromCurl.tsx(1 hunks)src/InstallCTA/NavTab.tsx(0 hunks)src/InstallCTA/TabSelector.tsx(2 hunks)src/InstallCTA/UseWithGitHubActions.tsx(1 hunks)src/InstallCTA/index.scss(2 hunks)src/InstallCTA/index.tsx(3 hunks)src/InstallCTA/types.ts(2 hunks)src/Link/index.scss(1 hunks)src/MacInstaller/index.scss(2 hunks)src/MacInstaller/index.tsx(1 hunks)src/Navigation/Navigation.stories.tsx(1 hunks)src/Navigation/index.scss(1 hunks)src/Navigation/index.tsx(1 hunks)src/PageLayout/PageLayout.stories.tsx(2 hunks)src/PageLayout/index.tsx(1 hunks)src/hooks/useHighlight.ts(3 hunks)src/hooks/useSystemColorScheme.ts(2 hunks)src/index.ts(1 hunks)src/sass/_mixins.scss(1 hunks)src/sass/_tokens.scss(2 hunks)
💤 Files with no reviewable changes (4)
- src/InstallCTA/NavTab.tsx
- src/CodeFile/index.scss
- src/CodeFile/index.tsx
- src/CodeFile/CodeFile.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (20)
- src/MacInstaller/index.tsx
- src/sass/_tokens.scss
- src/CodeBlock/index.tsx
- src/InstallCTA/TabSelector.tsx
- src/InstallCTA/InstallForNixOS.tsx
- src/Link/index.scss
- src/index.ts
- src/InstallCTA/UseWithGitHubActions.tsx
- src/CodeBlock/CodeBlock.stories.tsx
- package.json
- src/CodeBlock/index.scss
- src/hooks/useHighlight.ts
- src/InstallCTA/FindAwsAMIs.tsx
- src/Highlight/index.scss
- src/sass/_mixins.scss
- src/hooks/useSystemColorScheme.ts
- src/MacInstaller/index.scss
- src/InstallCTA/InstallFromCurl.tsx
- src/ColorProvider/index.tsx
- src/InstallCTA/index.scss
🧰 Additional context used
🧬 Code graph analysis (6)
src/Navigation/Navigation.stories.tsx (1)
src/Placeholder/index.tsx (1)
Placeholder(9-29)
src/PageLayout/PageLayout.stories.tsx (1)
src/Placeholder/index.tsx (1)
Placeholder(9-29)
src/Header/index.tsx (1)
src/index.ts (1)
HeaderProps(58-58)
src/Navigation/index.tsx (1)
src/index.ts (1)
NavigationProps(78-78)
src/InstallCTA/types.ts (1)
src/InstallCTA/index.tsx (1)
InstallTarget(17-17)
src/InstallCTA/InstallCTA.stories.tsx (1)
src/InstallCTA/index.tsx (1)
InstallTarget(17-17)
🪛 Biome (2.1.2)
src/Navigation/Navigation.stories.tsx
[error] 27-27: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 28-28: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 50-50: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 51-51: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 69-69: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 70-70: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 94-94: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 95-95: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/PageLayout/PageLayout.stories.tsx
[error] 55-55: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 64-64: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/InstallCTA/index.tsx
[error] 78-83: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (6)
src/InstallCTA/types.ts (1)
20-22: Confirm product intent: map Windows → Linux.If Windows users shouldn’t be funneled to Linux instructions, consider a neutral landing or explicit Windows guidance.
src/InstallCTA/index.tsx (1)
47-53: All mapped components accept an optionalversionprop.
MacInstaller declares MacInstallerProps{ version?: string }; InstallFromCurl, FindAwsAMIs, UseWithGitHubActions and InstallForNixOS useTabProps { version?: string }. No changes required.src/PageLayout/index.tsx (1)
26-32: LGTM: conditional rendering and per-item wrapping improves reconciliation.No functional concerns with the conditional panes rendering; this should reduce unnecessary DOM churn.
src/Header/index.tsx (1)
8-9: Public API tightening — verify downstream call sites & re-exportselements changed to React.ReactElement[] (src/Header/index.tsx); src/index.ts re-exports HeaderProps. Repo search found no in-repo usages of elements — verify stories and external consumers accept an array of React elements and update call-sites or types if they previously relied on ReactNode/other types.
src/Navigation/index.scss (1)
28-31: Confirm focus-visible styles on the toggle.Ensure
@include mixins.svg-button;provides a clear:focus-visibleoutline for keyboard users. If not, add an explicit focus style.src/Navigation/index.tsx (1)
32-41: A11y details look solid.Good use of useId, aria-controls/expanded, and hiding decorative SVG via
aria-hiddenandfocusable="false".
| const Header: FC<HeaderProps> = ({ logo, elements = [] }) => { | ||
| return ( | ||
| <header className="header" role="banner"> | ||
| <div className="header"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restore semantic landmark: use
(banner).Switching the root to a div loses the banner landmark. Use the semantic element (role optional).
- <div className="header">
+ <header className="header" role="banner">
...
- </div>
+ </header>Also applies to: 25-25
🤖 Prompt for AI Agents
In src/Header/index.tsx around lines 13 and 25 the root element was changed from
a semantic <header> (banner) to a <div>, removing the landmark; replace the <div
className="header"> (and the corresponding closing tag) with a semantic <header
className="header"> to restore the banner landmark, optionally add role="banner"
if you need explicit role support for older assistive tech, and ensure any
props/attributes and CSS selectors continue to work with the header element.
| children={[ | ||
| <Placeholder key="lorem" height="100%" label="Lorem" />, | ||
| <Placeholder key="ipsum" height="100%" label="Ipsum" />, | ||
| ]} | ||
| />, | ||
| ]} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t pass children prop explicitly; use JSX children.
Fixes the lint error and matches React conventions.
- <Navigation
- key="nav"
- children={[
- <Placeholder key="lorem" height="100%" label="Lorem" />,
- <Placeholder key="ipsum" height="100%" label="Ipsum" />,
- ]}
- />,
+ <Navigation key="nav">
+ <Placeholder key="lorem" height="100%" label="Lorem" />
+ <Placeholder key="ipsum" height="100%" label="Ipsum" />
+ </Navigation>,📝 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.
| children={[ | |
| <Placeholder key="lorem" height="100%" label="Lorem" />, | |
| <Placeholder key="ipsum" height="100%" label="Ipsum" />, | |
| ]} | |
| />, | |
| ]} | |
| <Navigation key="nav"> | |
| <Placeholder key="lorem" height="100%" label="Lorem" /> | |
| <Placeholder key="ipsum" height="100%" label="Ipsum" /> | |
| </Navigation>, | |
| ]} |
🧰 Tools
🪛 Biome (2.1.2)
[error] 55-55: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
🤖 Prompt for AI Agents
In src/PageLayout/PageLayout.stories.tsx around lines 55 to 60, the story is
passing children via an explicit children prop which triggers a lint error and
goes against React conventions; remove the children={...} prop and instead place
the Placeholder elements as normal JSX children inside the component tag
(keeping their key and props intact), e.g. collapse the children array and nest
the two <Placeholder ... /> elements between the component's opening and closing
tags so the story uses standard JSX children.
1 participant
Summary by CodeRabbit
New Features
Improvements
Reliability
Chores