Conversation
🦋 Changeset detectedLatest commit: 47b0472 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
📝 WalkthroughWalkthroughUpdates StudioCMS UI to support Astro v6, introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
commit: |
Test ResultsDetails
Suites300 passed, 0 failed, and 0 other
Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
Dokploy Preview Deployment
|
|
@Adammatthiesen Two things here:
The package itself works though! Feel free to use the preview release |
|
Co-authored-by: Adam Matthiesen <amatthiesen@outlook.com>
Followup, Knip is now working correctly just needed some cleanup in the repo from unused deps |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
docs/src/starlightOverrides/SiteTitle.astro (2)
61-61: Clarify the purpose of the empty div.This empty
<div></div>appears intentional, likely to satisfy thediv:not(:last-child)border selector at line 157-159. Consider adding a comment to clarify its purpose for future maintainers.📝 Add clarifying comment
- <div></div> + {/* Empty div to prevent border on last .main-links child */} + <div></div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/starlightOverrides/SiteTitle.astro` at line 61, The empty <div></div> is used intentionally to satisfy the CSS rule targeting div:not(:last-child) (so the previous elements get the border), so update the SiteTitle component to keep the empty div but add a clarifying inline comment immediately adjacent to it explaining its purpose (e.g., "placeholder to ensure div:not(:last-child) applies to previous items"); reference the empty element and the selector div:not(:last-child) in that comment so future maintainers understand why it must remain.
63-80: Duplicatealign-items: centerdeclaration and unusual style placement.
Lines 68 and 71 both declare
align-items: center- the second one is redundant.The
<style>block is nested inside the.site-title-quicklinksdiv (lines 63-223). While this works in Astro, placing styles at the component root level is more conventional and easier to maintain.♻️ Remove duplicate property
.site-title { justify-self: flex-start; display: flex; flex-direction: row; align-items: center; max-width: 100%; overflow: hidden; - align-items: center; color: var(--sl-color-accent-high);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/starlightOverrides/SiteTitle.astro` around lines 63 - 80, Remove the duplicate CSS property and move the style block out of the nested .site-title-quicklinks container: edit SiteTitle.astro to remove the redundant second "align-items: center" in the .site-title rule and relocate the entire <style> block to the component root (outside the .site-title-quicklinks div) so styles apply cleanly and are easier to maintain.docs/src/content/docs/docs/guides/customization.mdx (1)
73-96: Documentation fornoInjectResetCSSlooks good.The new section clearly explains the option and provides a useful example for manual inclusion via the virtual module.
Minor style suggestion: Line 90 starts with "If you want to" which repeats the phrasing from line 75. Consider varying the wording, e.g., "To include it on select pages manually...".
,
✏️ Optional phrasing improvement
-If you want to include it on select pages manually, you can use the `studiocms:ui/reset-css` virtual module. +To include the reset CSS on select pages manually, use the `studiocms:ui/reset-css` virtual module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/content/docs/docs/guides/customization.mdx` around lines 73 - 96, Minor wording repetition: replace the repeated "If you want to" phrasing to improve flow; update the sentence that currently begins "If you want to include it on select pages manually, you can use the `studiocms:ui/reset-css` virtual module." to a varied phrasing such as "To include it on select pages manually, use the `studiocms:ui/reset-css` virtual module." Keep the examples and code blocks for the `noInjectResetCSS` option and the import of "studiocms:ui/reset-css" unchanged—only alter the sentence text to the new wording to remove the duplicate phrasing.docs/astro.config.mts (1)
47-49: Document why reset CSS injection is disabled.Please add a brief inline comment here to capture the rationale (
noInjectResetCSS: true), since this is a non-default integration behavior and easy to accidentally revert later.Suggested small clarification
ui({ + // Keep Starlight/docs baseline styles untouched; avoid global reset conflicts. noInjectResetCSS: true, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/astro.config.mts` around lines 47 - 49, Add a short inline comment next to the ui({ noInjectResetCSS: true }) option explaining why reset CSS injection is disabled (e.g., to avoid conflicting global resets from our design system/other global stylesheet or because we inject a tailored reset elsewhere), so future maintainers won't accidentally revert the non-default noInjectResetCSS behavior; place the comment adjacent to the noInjectResetCSS property in the ui(...) configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/src/content/docs/docs/upgrade-guides/1.1-to-1.2.mdx`:
- Line 9: Fix the duplicate word in the sentence beginning "Version 1.2 of
StudioCMS UI brings support for Astro v6..." by removing the extra "with" so it
reads "...make the components with v6." Locate the string in the 1.1-to-1.2
upgrade guide (the line containing "No changes are required to make the
components with with v6.") and change "with with" to a single "with".
In `@packages/studiocms_ui/src/index.ts`:
- Around line 262-267: The current logic injects 'reset-css' whenever
options.noInjectResetCSS is false, even if options.noInjectCSS is true, which
breaks backward compatibility; update the injection condition so reset-css is
only injected when neither options.noInjectResetCSS nor options.noInjectCSS is
true (i.e., check both flags before calling injectScript for 'page-ssr'
`'studiocms:ui/reset-css'`), leaving the existing injectScript call for
`'studiocms:ui/global-css'` guarded by options.noInjectCSS as-is.
---
Nitpick comments:
In `@docs/astro.config.mts`:
- Around line 47-49: Add a short inline comment next to the ui({
noInjectResetCSS: true }) option explaining why reset CSS injection is disabled
(e.g., to avoid conflicting global resets from our design system/other global
stylesheet or because we inject a tailored reset elsewhere), so future
maintainers won't accidentally revert the non-default noInjectResetCSS behavior;
place the comment adjacent to the noInjectResetCSS property in the ui(...)
configuration.
In `@docs/src/content/docs/docs/guides/customization.mdx`:
- Around line 73-96: Minor wording repetition: replace the repeated "If you want
to" phrasing to improve flow; update the sentence that currently begins "If you
want to include it on select pages manually, you can use the
`studiocms:ui/reset-css` virtual module." to a varied phrasing such as "To
include it on select pages manually, use the `studiocms:ui/reset-css` virtual
module." Keep the examples and code blocks for the `noInjectResetCSS` option and
the import of "studiocms:ui/reset-css" unchanged—only alter the sentence text to
the new wording to remove the duplicate phrasing.
In `@docs/src/starlightOverrides/SiteTitle.astro`:
- Line 61: The empty <div></div> is used intentionally to satisfy the CSS rule
targeting div:not(:last-child) (so the previous elements get the border), so
update the SiteTitle component to keep the empty div but add a clarifying inline
comment immediately adjacent to it explaining its purpose (e.g., "placeholder to
ensure div:not(:last-child) applies to previous items"); reference the empty
element and the selector div:not(:last-child) in that comment so future
maintainers understand why it must remain.
- Around line 63-80: Remove the duplicate CSS property and move the style block
out of the nested .site-title-quicklinks container: edit SiteTitle.astro to
remove the redundant second "align-items: center" in the .site-title rule and
relocate the entire <style> block to the component root (outside the
.site-title-quicklinks div) so styles apply cleanly and are easier to maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c949aa9a-2570-4d21-8049-c3d9d8d00031
⛔ Files ignored due to path filters (4)
packages/studiocms_ui/test/components/__snapshots__/Sidebar.test.ts.snapis excluded by!**/*.snappackages/studiocms_ui/test/components/__snapshots__/Tabs.test.ts.snapis excluded by!**/*.snappackages/studiocms_ui/test/components/__snapshots__/User.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
.changeset/dry-carpets-drive.md.changeset/lemon-countries-repeat.md.github/workflows/ci-docker-main.ymldocs/astro.config.mtsdocs/package.jsondocs/src/content/docs/docs/components/progress.mdxdocs/src/content/docs/docs/components/skeleton.mdxdocs/src/content/docs/docs/components/tabs.mdxdocs/src/content/docs/docs/components/tooltip.mdxdocs/src/content/docs/docs/guides/customization.mdxdocs/src/content/docs/docs/upgrade-guides/0.4-to-1.0.mdxdocs/src/content/docs/docs/upgrade-guides/1.1-to-1.2.mdxdocs/src/content/docs/docs/utilities/prose.mdxdocs/src/plugins/rehypeAutolink.tsdocs/src/plugins/rehypePluginKit.tsdocs/src/starlightOverrides/Header.astrodocs/src/starlightOverrides/PageTitle.astrodocs/src/starlightOverrides/SiteTitle.astrodocs/src/styles/starlight.cssknip.config.tspackage.jsonpackages/studiocms_ui/package.jsonpackages/studiocms_ui/src/css/global.csspackages/studiocms_ui/src/index.tspackages/studiocms_ui/src/utils/headers.tspackages/studiocms_ui/test/components/Modal.test.tspackages/studiocms_ui/test/components/Row.test.tspackages/studiocms_ui/vitest.config.tspnpm-workspace.yaml
💤 Files with no reviewable changes (4)
- packages/studiocms_ui/src/css/global.css
- knip.config.ts
- docs/src/plugins/rehypeAutolink.ts
- .github/workflows/ci-docker-main.yml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…nto feat/astro-v6
Description
Summary by CodeRabbit
Release Notes
New Features
noInjectResetCSSconfiguration option to disable automatic CSS reset injection with manual import capabilityDocumentation