Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsCodeEditor (@astryxdesign/lab) · View in Storybook
Component Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: 1 accessibility violation(s) found — 1 serious. CodeEditor - 1 issue(s)
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
There was a problem hiding this comment.
Approval waits on a maintainer providing or identifying the governing spec for CodeEditor's new prebuilt appearance, settling whether the editor should gain the 1px outline, accent focus border and gutter divider its styles declare, or drop those declarations instead. The build side checks out: the prebuilt lab.css now carries the outline and divider, @astryxdesign/build compiles CodeEditor.tsx again, and Lab's build now fails on unsupported StyleX declarations the way Core's does. If the divider stays, please switch borderRightWidth/borderRightStyle/borderRightColor to their borderInlineEnd* equivalents, as Banner and DateRangeInput do: in right-to-left layouts the physical right border doubles the editor's outer edge instead of separating line numbers from code.
[Automated review]
| borderRightWidth: borderVars['--border-width'], | ||
| borderRightStyle: 'solid', | ||
| borderRightColor: colorVars['--color-border'], |
There was a problem hiding this comment.
If the divider stays, please make it logical. In right-to-left layouts the gutter sits on the right, so a physical right border lands on the editor's outer edge (doubling the root border) and nothing separates line numbers from code. Core's Banner and DateRangeInput use the same logical longhands:
borderInlineEndWidth: borderVars['--border-width'],
borderInlineEndStyle: 'solid',
borderInlineEndColor: colorVars['--color-border'],There was a problem hiding this comment.
Done in ddb9888: the divider now uses the borderInlineEnd* longhands. In right-to-left layouts it sits on the gutter's inner edge (RTL still added to the description). Left-to-right pixels are unchanged, and lab.css stays byte-identical to main's.
There was a problem hiding this comment.
Approval waits on a maintainer providing or identifying the governing spec for CodeEditor's prebuilt appearance: whether the editor should gain the 1px outline, the accent focus border and the line-number divider its styles declare, or drop those declarations and stay borderless. The build side checks out: @astryxdesign/build now compiles CodeEditor.tsx, lab.css is unchanged while the compiled editor uses rules it already held, and Lab's build rejects unsupported StyleX declarations as Core's does. The right-to-left divider now sits between the line numbers and the code, which resolves the earlier suggestion. Nothing else is needed from you unless a maintainer chooses the borderless option.
[Automated review]
ddb9888 to
8621ac4
Compare
CodeEditor declared its outline and gutter divider as `border` and `borderRight` shorthands, which StyleX's default style resolution does not support. The prebuilt lab.css, compiled in StyleX's silent mode, dropped both, and source builds, which validate with `throw`, failed with "CodeEditor.tsx: border is not supported" whenever Lab was installed. Write them as longhands, the divider as a logical inline-end border so it separates the line numbers from the code in right-to-left layouts too, and make Lab's build reject unsupported declarations the way Core's does, with a regression test.
9873d6b to
ecb72b4
Compare
There was a problem hiding this comment.
Approval waits on one owner decision: should Lab's prebuilt CodeEditor gain the 1px outline, the accent focus border and the line-number divider its styles declare, or should the root and gutter border declarations be removed so it stays borderless? No current spec covers CodeEditor's appearance, so an engineering owner records the decision to keep the borders by commenting /approve-spec <full head SHA> on this pull request, and you may record it yourself as an engineering owner; choosing the borderless editor instead means deleting those two declarations. The build side checks out: @astryxdesign/build compiles CodeEditor.tsx again, lab.css is unchanged, the right-to-left divider sits between the line numbers and the code, and Lab's build now rejects unsupported StyleX declarations as Core's does. Nothing else is needed.
[Automated review]
User impact
Why: Apps on the documented Next.js source build fail once
@astryxdesign/labis installed, even if they never import it. One CodeEditor border style stops the build. The prebuiltlab.cssdrops that style, so the editor draws none of its three declared borders.What led to this: We are moving one of our products onto Astryx source builds. While adding Turbopack support (#7754), the Next.js example plus Lab would not build.
What changes: The outline and divider use longhand border properties, which both builds accept. Lab's build now rejects unsupported shorthands, as Core's does.
Visual authority
DDrow reused or amended.Design decisionstable by a team design owner.docs/design/.No record covers CodeEditor's appearance, so its owners decide: keep the borders or remove them.
Longhands fix the style at its source and keep that check.
spec:AST-080FR7 (current) asks both builds for the same rules; today one throws and the other drops them.Before and after
Prebuilt stylesheets only (Core, Lab, neutral theme); Chromium, line numbers on.
State matrix
Light and dark: the outline and divider go from none to 1px in the border color.
Focused: before, only the focus ring; after, the accent border plus the ring.
The editor grows 1px on each side.
Right to left: the divider sits between the numbers and the code:
Scope
Testing
lab.cssis byte-identical to main's: the longhands reuse rules the sheet already has, so only the editor's class list changes.