refactor: standardize component spacing, typography, and border-radius#13
Open
pharmacist-sabot wants to merge 6 commits intomainfrom
Open
refactor: standardize component spacing, typography, and border-radius#13pharmacist-sabot wants to merge 6 commits intomainfrom
pharmacist-sabot wants to merge 6 commits intomainfrom
Conversation
Reviewer's GuideRefactors global and component-level styling to use centralized design tokens (spacing, typography, radii) with CSS layers, improves accessibility and motion preferences, and standardizes visual hierarchy across pages and UI components. Flow diagram for non-blocking Google Fonts loading and typography tokensgraph TD
browser["Browser loads BaseLayout.astro"] --> head_links["<head> with preconnect and font stylesheet links"]
head_links --> preconnect1["link rel=preconnect fonts.googleapis.com"]
head_links --> preconnect2["link rel=preconnect fonts.gstatic.com crossorigin"]
head_links --> font_css_link["link rel=stylesheet href=GoogleFonts media=print onload"]
head_links --> noscript_link["noscript fallback stylesheet link"]
font_css_link -->|onload| switch_media["onload handler sets media='all'"]
switch_media --> fonts_active["Prompt and JetBrainsMono available"]
noscript_link -->|if JS disabled| fonts_active
fonts_active --> typography_tokens[":root font-family and fluid text scale tokens"]
typography_tokens --> base_body["@layer base body font-size var(--text-base)"]
typography_tokens --> headings["@layer base headings h1..h6 use var(--text-*)"]
typography_tokens --> components["Components use var(--text-sm\|base\|lg\|xl\|2xl\|3xl)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 3 minor |
TIP This summary will be updated as you push new changes. Give us feedback
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Several top-level headings were changed from
to lower levels (e.g., about, blog index, tools, INVS/HOSxP pages); consider keeping a single semantic
per page for accessibility/SEO and adjusting the visual size via CSS instead of demoting the heading level.
- The terminal code block styles claim all values are tokenized via CSS custom properties, but hard-coded colors and sizes (e.g., #1e1e1e, #2d2d2d, 40px) are still present; either move these into the design tokens or component-level CSS vars, or relax the comment so it matches the implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several top-level headings were changed from <h1> to lower levels (e.g., about, blog index, tools, INVS/HOSxP pages); consider keeping a single semantic <h1> per page for accessibility/SEO and adjusting the visual size via CSS instead of demoting the heading level.
- The terminal code block styles claim all values are tokenized via CSS custom properties, but hard-coded colors and sizes (e.g., #1e1e1e, #2d2d2d, 40px) are still present; either move these into the design tokens or component-level CSS vars, or relax the comment so it matches the implementation.
## Individual Comments
### Comment 1
<location path="src/pages/about.astro" line_range="9-13" />
<code_context>
<div class="text-content">
- <h1>จากเภสัชกรสู่โปรแกรมเมอร์</h1>
- <p class="lead">
+ <h3>จากเภสัชกรสู่โปรแกรมเมอร์</h3>
+ <p>
เส้นทางของผมเริ่มต้นที่เคาน์เตอร์จ่ายยาท่ามกลางใบสั่งยาและการดูแลผู้ป่วย
</code_context>
<issue_to_address>
**issue:** Demoting primary headings to h3 risks breaking the document heading hierarchy and accessibility.
On pages like about, INVS, HOSxP, blog index, tools index, and tool detail, the main titles are now `h3`/`h4` instead of `h1`/`h2`. If these are the primary page headings, this removes or hides the `h1` from the outline, which harms screen reader navigation and SEO. Keep the correct semantic levels for document structure and use the typography tokens (e.g. `font-size: var(--text-xl)`) to adjust visual size instead of changing heading levels for styling only.
</issue_to_address>
### Comment 2
<location path="src/styles/terminal.css" line_range="9-18" />
<code_context>
/* Layout: Inline Block (Stable) */
position: relative;
display: flex;
- gap: 1.25rem;
</code_context>
<issue_to_address>
**suggestion:** Terminal code block styles still hardcode colors instead of using the new design tokens.
This file’s header says “All values tokenized via CSS custom properties”, but this block still hardcodes colors (e.g. `#1e1e1e`, `#f8fafc`, `rgba(255, 255, 255, 0.1)`, `rgba(0, 0, 0, 0.3)`) instead of using the tokens defined in `:root`. To keep theming centralized and follow the “never hardcode” guideline in `global.css`, please add terminal-specific background/border/shadow tokens in `:root` and reference those here.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+9
to
+18
| @layer components { | ||
| /* --- Code block container: Terminal Look --- */ | ||
| .prose pre { | ||
| margin-left: -1rem; | ||
| margin-right: -1rem; | ||
| border-radius: 0; | ||
| padding: 3rem 1rem 1rem 1rem; | ||
| font-size: 0.85rem; | ||
| position: relative; | ||
| background: #1e1e1e; | ||
| color: #f8fafc; | ||
| border: 1px solid rgba(255, 255, 255, 0.1); | ||
| border-radius: var(--radius-md); | ||
| padding: 3.5rem var(--space-6) var(--space-6) var(--space-6); | ||
| margin: var(--space-8) 0; |
There was a problem hiding this comment.
suggestion: Terminal code block styles still hardcode colors instead of using the new design tokens.
This file’s header says “All values tokenized via CSS custom properties”, but this block still hardcodes colors (e.g. #1e1e1e, #f8fafc, rgba(255, 255, 255, 0.1), rgba(0, 0, 0, 0.3)) instead of using the tokens defined in :root. To keep theming centralized and follow the “never hardcode” guideline in global.css, please add terminal-specific background/border/shadow tokens in :root and reference those here.
📝 WalkthroughWalkthroughThis PR replaces many hardcoded CSS values across components, pages, and global styles with design-system CSS variables, introduces CSS cascade layers, adds focus-visible and prefers-reduced-motion rules, and adds non-blocking font preconnect/load optimizations in BaseLayout. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
src/pages/tools/cupsabot.astro (1)
13-13: Consider preserving semantic heading hierarchy (h1 for page title, h2 for section).The visual sizing can still be controlled via CSS classes/tokens, while keeping stronger document structure for accessibility/SEO.
Also applies to: 37-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/tools/cupsabot.astro` at line 13, Replace the top-level visual title element <h3>ระบบเบิกจ่ายยาออนไลน์</h3> with a semantic <h1> for the page title and change the other section heading at the second occurrence (currently an <h3> at line 37) to <h2>; keep any CSS classes or utility classes that control visual size so appearance is unchanged while ensuring correct semantic hierarchy using the <h1> for the main title and <h2> for subsections.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/about.astro`:
- Around line 78-82: The CSS block defining rules for the selector "h2" is
orphaned because the page uses only "h3" headings; either change the selector
from h2 to h3 in the CSS block so the divider styling applies to the existing
<h3> elements (update the rule that currently reads "h2 { ... }" to "h3 { ...
}"), or delete the entire h2 rule block if the divider styling is no longer
required.
- Line 9: The page uses an <h3> as the top-level heading which violates heading
hierarchy; change the heading in src/pages/about.astro from
<h3>จากเภสัชกรสู่โปรแกรมเมอร์</h3> to an <h1> so the page has a single primary
heading, and update the subsequent "contact channels" section headings to <h2>
to maintain proper hierarchy; if you prefer the layout to provide the <h1>,
ensure BaseLayout injects a single page title (check BaseLayout) and avoid
adding another <h1> in about.astro to prevent duplicates.
In `@src/pages/blog/index.astro`:
- Line 39: The blog page CSS adds padding-top: 4rem which duplicates the
`.main-content` spacing applied by BaseLayout; remove or reduce the top padding
in the blog page (the rule that sets `padding: 4rem 0 0;`) so it doesn't stack
with the `.main-content` padding in BaseLayout — either delete that declaration
or set the top padding to 0 (keep other axis spacing as needed) to avoid the
doubled gap.
- Line 20: The heading tag was changed to <h3> but the stylesheet still targets
.blog-header h1, so update the selector or the tag to match; either change the
markup in index.astro back to <h1> or update the CSS rule .blog-header h1 to
.blog-header h3 (or a more generic .blog-header h1, .blog-header h2,
.blog-header h3) so the intended spacing/styling applies to the new <h3>
heading.
In `@src/pages/hosxp.astro`:
- Line 7: The page's top-level heading is currently an h3; change the <h3
class="page-title">HOSxP</h3> element to an h1 so the page title is the primary
heading (e.g., replace the h3 tag with h1 while keeping class="page-title" and
the "HOSxP" text) to restore correct heading hierarchy given that BaseLayout
does not provide a higher-level page heading.
In `@src/pages/invs.astro`:
- Line 14: The page heading is currently an h3 element (<h3
class="page-title">INVS</h3>) which demotes the top-level heading; change that
element to an h1 while keeping the .page-title class (i.e., replace the h3 with
h1 for the INVS title) so the page has a proper top-level heading while
retaining existing styling.
In `@src/pages/tools/herbs-app.astro`:
- Line 13: The page title currently uses an h3 ("<h3>บัญชียาสมุนไพร
รพ.สระโบสถ์</h3>") which demotes the document heading baseline and the other
headings later in the file—update the semantic levels so the main page title is
an h1 and the subsequent section headings use h2 (and subsections h3) as
appropriate (also adjust the other headings referenced in the file that were
downgraded around the same area), and if you need a different visual size
control presentation use CSS classes to scale the headings rather than changing
their semantic level.
In `@src/pages/tools/index.astro`:
- Line 8: The page header now uses an <h3> element but the stylesheet still
targets .page-header h1, so update the selector to match the new heading level
(change .page-header h1 to .page-header h3) or revert the template heading to
<h1>; ensure the styles applied to .page-header (margin, typography) are applied
to the actual heading element (<h3> or <h1>) referenced in
src/pages/tools/index.astro.
In `@src/styles/global.css`:
- Line 331: Replace the deprecated CSS rule "word-break: break-word" with the
standard "overflow-wrap: anywhere" wherever it appears (e.g., the global
occurrence shown and the .prose code selector), i.e., remove the deprecated
property and add overflow-wrap: anywhere to preserve the same breaking behavior
in selectors like .prose code and the global rule.
- Line 314: Replace the deprecated CSS declaration "word-break: break-word" with
the standard property "overflow-wrap: break-word": locate the CSS rule
containing the "word-break: break-word" declaration and remove that line, then
add "overflow-wrap: break-word" (and optionally "word-break: normal" for
clarity) in the same rule so long words still wrap correctly.
- Around line 275-280: Remove the deprecated redundant property from the .prose
rule: delete the legacy "word-wrap" declaration since "overflow-wrap:
break-word" is already present; update the CSS rule for the .prose selector (the
block containing max-width, margin, overflow-wrap) to only include overflow-wrap
and remove word-wrap.
---
Nitpick comments:
In `@src/pages/tools/cupsabot.astro`:
- Line 13: Replace the top-level visual title element
<h3>ระบบเบิกจ่ายยาออนไลน์</h3> with a semantic <h1> for the page title and
change the other section heading at the second occurrence (currently an <h3> at
line 37) to <h2>; keep any CSS classes or utility classes that control visual
size so appearance is unchanged while ensuring correct semantic hierarchy using
the <h1> for the main title and <h2> for subsections.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2bd165c9-0ff8-4d05-8b21-326432f30a54
📒 Files selected for processing (27)
src/components/blog/BlogPostCard.astrosrc/components/blog/CategoryCard.astrosrc/components/blog/RelatedPosts.astrosrc/components/blog/ShareButtons.astrosrc/components/content/CodeExplainer.astrosrc/components/content/GitCommand.astrosrc/components/content/Image.astrosrc/components/content/InfoBox.astrosrc/components/content/SideNote.astrosrc/components/content/Table.astrosrc/components/content/YouTube.astrosrc/components/layout/Navbar.astrosrc/components/tools/ToolCard.astrosrc/components/ui/FeatureCard.astrosrc/components/ui/Search.astrosrc/layouts/BaseLayout.astrosrc/layouts/BlogPostLayout.astrosrc/pages/about.astrosrc/pages/blog/index.astrosrc/pages/hosxp.astrosrc/pages/index.astrosrc/pages/invs.astrosrc/pages/tools/cupsabot.astrosrc/pages/tools/herbs-app.astrosrc/pages/tools/index.astrosrc/styles/global.csssrc/styles/terminal.css
Comment on lines
78
to
82
| h2 { | ||
| margin-top: 2.5rem; | ||
| margin-top: var(--space-10); | ||
| border-top: 1px solid var(--card-border-color); | ||
| padding-top: 2rem; | ||
| padding-top: var(--space-8); | ||
| } |
There was a problem hiding this comment.
Orphaned CSS selector: h2 rule targets non-existent elements.
This h2 selector applies styling for section dividers, but the page markup contains only <h3> elements (lines 9 and 33). This CSS rule will never match anything on this page.
Either remove this dead CSS or update the selector to target the actual heading level used.
Proposed fix (if keeping h3 headings)
- h2 {
+ h3:not(:first-of-type) {
margin-top: var(--space-10);
border-top: 1px solid var(--card-border-color);
padding-top: var(--space-8);
}Or remove the block entirely if the divider styling is no longer 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.
Suggested change
| h2 { | |
| margin-top: 2.5rem; | |
| margin-top: var(--space-10); | |
| border-top: 1px solid var(--card-border-color); | |
| padding-top: 2rem; | |
| padding-top: var(--space-8); | |
| } | |
| h3:not(:first-of-type) { | |
| margin-top: var(--space-10); | |
| border-top: 1px solid var(--card-border-color); | |
| padding-top: var(--space-8); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/about.astro` around lines 78 - 82, The CSS block defining rules for
the selector "h2" is orphaned because the page uses only "h3" headings; either
change the selector from h2 to h3 in the CSS block so the divider styling
applies to the existing <h3> elements (update the rule that currently reads "h2
{ ... }" to "h3 { ... }"), or delete the entire h2 rule block if the divider
styling is no longer required.
| max-width: 700px; | ||
| margin-left: auto; | ||
| margin-right: auto; | ||
| padding: 4rem 0 0; |
There was a problem hiding this comment.
Top padding likely doubles with BaseLayout spacing.
Line 39 adds padding-top: 4rem, and src/layouts/BaseLayout.astro already applies padding: 6rem 0 4rem on .main-content (Line 82 there). This likely introduces an overly large top gap on the blog page.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/blog/index.astro` at line 39, The blog page CSS adds padding-top:
4rem which duplicates the `.main-content` spacing applied by BaseLayout; remove
or reduce the top padding in the blog page (the rule that sets `padding: 4rem 0
0;`) so it doesn't stack with the `.main-content` padding in BaseLayout — either
delete that declaration or set the top padding to 0 (keep other axis spacing as
needed) to avoid the doubled gap.
src/pages/tools/herbs-app.astro
Outdated
| <div class="project-showcase"> | ||
| <header class="project-header"> | ||
| <h1>บัญชียาสมุนไพร รพ.สระโบสถ์</h1> | ||
| <h3>บัญชียาสมุนไพร รพ.สระโบสถ์</h3> |
There was a problem hiding this comment.
Keep the semantic heading levels here.
Line 13 demotes the page title to h3, and Line 49 plus Line 78 continue from that lower baseline, so the document now starts at level 3 and skips h2. For a styling refactor, keep the heading levels semantic and use CSS to control the visual scale instead.
Suggested fix
- <h3>บัญชียาสมุนไพร รพ.สระโบสถ์</h3>
+ <h1>บัญชียาสมุนไพร รพ.สระโบสถ์</h1>
...
- <h3>คุณสมบัติหลัก</h3>
+ <h2>คุณสมบัติหลัก</h2>
...
- <h4>Tech Stack</h4>
+ <h3>Tech Stack</h3>Also applies to: 49-49, 78-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/tools/herbs-app.astro` at line 13, The page title currently uses an
h3 ("<h3>บัญชียาสมุนไพร รพ.สระโบสถ์</h3>") which demotes the document heading
baseline and the other headings later in the file—update the semantic levels so
the main page title is an h1 and the subsequent section headings use h2 (and
subsections h3) as appropriate (also adjust the other headings referenced in the
file that were downgraded around the same area), and if you need a different
visual size control presentation use CSS classes to scale the headings rather
than changing their semantic level.
Comment on lines
+275
to
+280
| .prose { | ||
| max-width: 800px; | ||
| margin: 0 auto; | ||
| overflow-wrap: break-word; | ||
| word-wrap: break-word; | ||
| } |
There was a problem hiding this comment.
Remove deprecated word-wrap property.
The word-wrap property is a legacy alias for overflow-wrap. Since overflow-wrap: break-word is already present on line 278, word-wrap on line 279 is redundant.
Proposed fix
.prose {
max-width: 800px;
margin: 0 auto;
overflow-wrap: break-word;
- word-wrap: break-word;
}📝 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.
Suggested change
| .prose { | |
| max-width: 800px; | |
| margin: 0 auto; | |
| overflow-wrap: break-word; | |
| word-wrap: break-word; | |
| } | |
| .prose { | |
| max-width: 800px; | |
| margin: 0 auto; | |
| overflow-wrap: break-word; | |
| } |
🧰 Tools
🪛 Stylelint (17.6.0)
[error] 279-279: Expected "word-wrap" to be "overflow-wrap" (property-no-deprecated)
(property-no-deprecated)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/styles/global.css` around lines 275 - 280, Remove the deprecated
redundant property from the .prose rule: delete the legacy "word-wrap"
declaration since "overflow-wrap: break-word" is already present; update the CSS
rule for the .prose selector (the block containing max-width, margin,
overflow-wrap) to only include overflow-wrap and remove word-wrap.
| padding: 0.2em 0.4em; | ||
| border-radius: var(--radius-sm); | ||
| font-size: 0.9em; | ||
| word-break: break-word; |
There was a problem hiding this comment.
Replace deprecated word-break: break-word with standard property.
word-break: break-word is deprecated. Use overflow-wrap: break-word instead for breaking long words.
Proposed fix
.prose code {
font-family: var(--font-mono);
background-color: var(--card-border-color);
color: var(--brand-color);
padding: 0.2em 0.4em;
border-radius: var(--radius-sm);
font-size: 0.9em;
- word-break: break-word;
+ overflow-wrap: break-word;
}📝 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.
Suggested change
| word-break: break-word; | |
| overflow-wrap: break-word; |
🧰 Tools
🪛 Stylelint (17.6.0)
[error] 314-314: Unexpected deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/styles/global.css` at line 314, Replace the deprecated CSS declaration
"word-break: break-word" with the standard property "overflow-wrap: break-word":
locate the CSS rule containing the "word-break: break-word" declaration and
remove that line, then add "overflow-wrap: break-word" (and optionally
"word-break: normal" for clarity) in the same rule so long words still wrap
correctly.
| line-height: 1.6; | ||
| font-weight: 400; | ||
| white-space: pre-wrap; | ||
| word-break: break-word; |
There was a problem hiding this comment.
Replace deprecated word-break: break-word with standard property.
Same issue as in .prose code — word-break: break-word is deprecated.
Proposed fix
.prose pre {
...
white-space: pre-wrap;
- word-break: break-word;
+ overflow-wrap: break-word;
}📝 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.
Suggested change
| word-break: break-word; | |
| overflow-wrap: break-word; |
🧰 Tools
🪛 Stylelint (17.6.0)
[error] 331-331: Unexpected deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/styles/global.css` at line 331, Replace the deprecated CSS rule
"word-break: break-word" with the standard "overflow-wrap: anywhere" wherever it
appears (e.g., the global occurrence shown and the .prose code selector), i.e.,
remove the deprecated property and add overflow-wrap: anywhere to preserve the
same breaking behavior in selectors like .prose code and the global rule.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/styles/global.css (1)
271-327:⚠️ Potential issue | 🟡 MinorReplace deprecated wrapping declarations in
.prosecode styles.Line 275 (
word-wrap: break-word) and Lines 310/327 (word-break: break-word) are deprecated and still trigger lint failures. Useoverflow-wrapinstead.Proposed fix
.prose { max-width: var(--prose-max-width); margin: 0 auto; overflow-wrap: break-word; - word-wrap: break-word; } .prose code { @@ - word-break: break-word; + overflow-wrap: break-word; } @@ .prose pre { @@ - word-break: break-word; + overflow-wrap: anywhere; }#!/bin/bash # Verify deprecated declarations are removed from this file rg -n 'word-wrap:\s*break-word|word-break:\s*break-word' src/styles/global.css🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles/global.css` around lines 271 - 327, The CSS uses deprecated declarations "word-wrap: break-word" and "word-break: break-word" inside the .prose selector rules (notably in .prose block and .prose pre/.prose code) which triggers lint failures; replace those occurrences with the modern "overflow-wrap: break-word" (and remove the deprecated properties) so that .prose, .prose code, and .prose pre only use overflow-wrap for wrapping behavior while preserving existing overflow-x/white-space settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/invs.astro`:
- Around line 38-42: The .invs-container rule is using shorthand padding
(padding: var(--space-12) 0) which resets left/right gutters from the shared
.container; update the rule for .invs-container to only set block-axis padding
(e.g., use padding-block: var(--space-12) or separate padding-top and
padding-bottom) so you preserve the .container horizontal spacing while keeping
the same vertical padding.
---
Duplicate comments:
In `@src/styles/global.css`:
- Around line 271-327: The CSS uses deprecated declarations "word-wrap:
break-word" and "word-break: break-word" inside the .prose selector rules
(notably in .prose block and .prose pre/.prose code) which triggers lint
failures; replace those occurrences with the modern "overflow-wrap: break-word"
(and remove the deprecated properties) so that .prose, .prose code, and .prose
pre only use overflow-wrap for wrapping behavior while preserving existing
overflow-x/white-space settings.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a7bcd74-7e39-4a5d-8ea0-5220673bec6b
📒 Files selected for processing (10)
src/components/layout/Navbar.astrosrc/layouts/BlogPostLayout.astrosrc/pages/about.astrosrc/pages/blog/index.astrosrc/pages/hosxp.astrosrc/pages/invs.astrosrc/pages/tools/cupsabot.astrosrc/pages/tools/herbs-app.astrosrc/pages/tools/index.astrosrc/styles/global.css
✅ Files skipped from review due to trivial changes (4)
- src/pages/tools/index.astro
- src/pages/tools/cupsabot.astro
- src/pages/hosxp.astro
- src/layouts/BlogPostLayout.astro
🚧 Files skipped from review as they are similar to previous changes (3)
- src/pages/blog/index.astro
- src/pages/tools/herbs-app.astro
- src/pages/about.astro
Comment on lines
+38
to
42
| .invs-container { | ||
| text-align: center; | ||
| padding: 4rem 0; | ||
| max-width: 900px; | ||
| margin: 0 auto; | ||
| /* .container provides max-width and centering */ | ||
| padding: var(--space-12) 0; | ||
| } |
There was a problem hiding this comment.
Avoid overriding .container horizontal gutter with shorthand padding.
Line 41 resets left/right padding to 0, which cancels the shared .container side spacing. Use block-axis padding only.
Proposed fix
.invs-container {
text-align: center;
/* .container provides max-width and centering */
- padding: var(--space-12) 0;
+ padding-block: var(--space-12);
}📝 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.
Suggested change
| .invs-container { | |
| text-align: center; | |
| padding: 4rem 0; | |
| max-width: 900px; | |
| margin: 0 auto; | |
| /* .container provides max-width and centering */ | |
| padding: var(--space-12) 0; | |
| } | |
| .invs-container { | |
| text-align: center; | |
| /* .container provides max-width and centering */ | |
| padding-block: var(--space-12); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/invs.astro` around lines 38 - 42, The .invs-container rule is using
shorthand padding (padding: var(--space-12) 0) which resets left/right gutters
from the shared .container; update the rule for .invs-container to only set
block-axis padding (e.g., use padding-block: var(--space-12) or separate
padding-top and padding-bottom) so you preserve the .container horizontal
spacing while keeping the same vertical padding.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce a centralized design token system and layered global stylesheet, and update components and pages to use standardized spacing, typography, and border-radius tokens.
Enhancements:
Summary by CodeRabbit
Refactor
Accessibility
Performance