feat: style improvements - #159
Conversation
✅ Deploy Preview for felixs-homepage ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds shared external-link indicators, refreshed landing and content components, a page-header primitive, updated themes and typography, responsive hero and 404 visuals, enhanced navigation behavior, page descriptions, and broader hover and reduced-motion styling. ChangesShared UI primitives
Content cards and link indicators
Landing content sections
Hero, error page, and theme visuals
Navigation and layout interactions
Page headers and descriptions
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/layouts/Header.astro (1)
148-167: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicated close-menu logic between button handler and
closeMenu().The button click handler (lines 148-153) duplicates the same close animation logic that
closeMenu()(lines 160-167) encapsulates. CallcloseMenu()in theifbranch to keep a single source of truth.♻️ Proposed refactor
button?.addEventListener("click", (event) => { // Prevent the same click from immediately closing the menu via the // document-level click-outside handler below. event.stopPropagation(); if (headerInner?.classList.contains("is-open")) { - headerInner.classList.remove("is-open"); - headerInner.classList.add("is-closing"); - setTimeout(() => { - headerInner.classList.remove("is-closing"); - }, 500); + closeMenu(); } else { headerInner?.classList.remove("is-closing"); headerInner?.classList.add("is-open"); } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/layouts/Header.astro` around lines 148 - 167, Replace the duplicated close-animation statements in the header button handler’s open-state if branch with a call to closeMenu(). Preserve the existing open-state behavior in the else branch and keep closeMenu() as the single implementation of menu closing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/events/EventCard.astro`:
- Around line 218-234: Extend the existing prefers-reduced-motion media query to
include .event-external, disabling its transition and hover transform while
preserving the current non-reduced-motion styling and hover color behavior.
In `@src/components/landing/ReadingSummary.astro`:
- Around line 44-113: Update the favorites mapping around the
`summary.favorites.map` callback to include a stable per-book index or unique
identifier, and use that value with the star index when constructing each
half-star gradient ID and matching `url(#...)` reference. Ensure every rendered
favorite book receives unique SVG IDs, including books without a `hiveId`.
In `@src/components/utils/ActionLink.astro`:
- Around line 20-44: Replace the duplicated inline SVG in the isExternal branch
of ActionLink with the shared ExternalIcon component, preserving its existing
external-link behavior and accessibility attributes. Reuse the component’s
established sizing and styling interface rather than maintaining separate icon
markup.
In `@src/layouts/Footer.astro`:
- Around line 185-191: Update the reduced-motion block in Footer.astro to also
disable the hover transforms for .text-links a:hover and .icon-links a:hover,
matching the existing Breadcrumbs pattern while preserving the normal hover
behavior for users without reduced-motion enabled.
---
Outside diff comments:
In `@src/layouts/Header.astro`:
- Around line 148-167: Replace the duplicated close-animation statements in the
header button handler’s open-state if branch with a call to closeMenu().
Preserve the existing open-state behavior in the else branch and keep
closeMenu() as the single implementation of menu closing.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5e1ab370-1804-4af1-9a9b-9214bf1d9b69
📒 Files selected for processing (24)
src/components/blog/PostItem.astrosrc/components/blog/YearGroup.astrosrc/components/books/Book.astrosrc/components/events/EventCard.astrosrc/components/landing/Hero.astrosrc/components/landing/ReadingSummary.astrosrc/components/landing/RecentBlueskyPosts.astrosrc/components/landing/RecentPosts.astrosrc/components/landing/UpcomingEvents.astrosrc/components/projects/Card.astrosrc/components/projects/Projects.astrosrc/components/utils/ActionLink.astrosrc/components/utils/Badge.astrosrc/components/utils/ExternalIcon.astrosrc/components/utils/IntroBadge.astrosrc/components/utils/List.astrosrc/components/utils/Section.astrosrc/components/utils/SectionSkeleton.astrosrc/layouts/Breadcrumbs.astrosrc/layouts/Footer.astrosrc/layouts/Header.astrosrc/pages/404.astrosrc/pages/index.astrosrc/styles/global.css
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/landing/RecentBlueskyPosts.astro`:
- Around line 45-48: Move ExternalIcon out of the clamped .post-text span and
make it a sibling within the post content container, following the structure
used by RecentPosts.astro and UpcomingEvents.astro. Update the corresponding
.item-main styling so it uses flex layout and keeps the icon aligned and visible
while only the text remains line-clamped.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 46947a66-33af-4776-838e-a95edaabf843
📒 Files selected for processing (12)
src/components/events/EventCard.astrosrc/components/landing/ReadingSummary.astrosrc/components/landing/RecentBlueskyPosts.astrosrc/components/landing/RecentPosts.astrosrc/components/landing/UpcomingEvents.astrosrc/components/utils/PageHeader.astrosrc/components/utils/Section.astrosrc/pages/blog.astrosrc/pages/books.astrosrc/pages/events.astrosrc/pages/index.astrosrc/pages/projects.astro
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/layouts/Footer.astro (1)
180-191: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate redundant hover styles.
The hover states for
.text-links aand.icon-links ashare the exact same properties. They can be combined to keep the CSS concise and DRY.♻️ Proposed refactor
.text-links a:hover, .icon-links a:hover { color: var(--text); + transform: translateY(-2px); } - - .text-links a:hover { - transform: translateY(-2px); - } - - .icon-links a:hover { - transform: translateY(-2px); - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/layouts/Footer.astro` around lines 180 - 191, Consolidate the duplicate transform rules for .text-links a:hover and .icon-links a:hover into a single shared selector, preserving the existing translateY(-2px) behavior and leaving the color hover rule unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/landing/ReadingSummary.astro`:
- Around line 34-37: Add role="img" to the star-rating span in
ReadingSummary.astro so its existing aria-label is exposed and announced by
screen readers, preserving the current `${stars} out of 5 stars` accessible
name.
In `@src/components/utils/Breadcrumbs.astro`:
- Around line 53-56: Update the reduced-motion styles for .back-link and .arrow
in Breadcrumbs.astro to remove motion while preserving transitions for
non-motion properties such as box-shadow, background, and border-color,
following the existing Footer.astro pattern. Replace the blanket transition:none
declaration with explicit non-motion transition definitions.
---
Outside diff comments:
In `@src/layouts/Footer.astro`:
- Around line 180-191: Consolidate the duplicate transform rules for .text-links
a:hover and .icon-links a:hover into a single shared selector, preserving the
existing translateY(-2px) behavior and leaving the color hover rule unchanged.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: b0728124-9f1d-467b-8a0b-be3c22c8aa23
📒 Files selected for processing (11)
src/components/blog/PostItem.astrosrc/components/events/EventCard.astrosrc/components/landing/Hero.astrosrc/components/landing/ReadingSummary.astrosrc/components/landing/RecentPosts.astrosrc/components/landing/UpcomingEvents.astrosrc/components/utils/ActionLink.astrosrc/components/utils/Breadcrumbs.astrosrc/components/utils/IntroBadge.astrosrc/layouts/Footer.astrosrc/styles/global.css
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/landing/ReadingSummary.astro`:
- Around line 153-156: Update the .row-icon rule in ReadingSummary.astro to use
a global selector so it applies to the SVG rendered by ExternalIcon.astro, while
preserving its existing color and flex-shrink styles.
In `@src/pages/blog.astro`:
- Around line 12-21: Replace the duplicated title/description markup with the
shared PageHeader component and remove the page-local .description styles. In
src/pages/blog.astro lines 12-21, use PageHeader with title “Blog Posts” and its
description slot; apply the same change in src/pages/books.astro lines 12-21
with “Bookshelf”, src/pages/events.astro lines 12-21 with “Events”, and
src/pages/projects.astro lines 11-20 with “Open Source”, preserving each
existing description content.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: db800cce-5743-4aee-951e-da098806a399
📒 Files selected for processing (5)
src/components/landing/ReadingSummary.astrosrc/pages/blog.astrosrc/pages/books.astrosrc/pages/events.astrosrc/pages/projects.astro
github-actions
Bot
added
the
📦 package
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/utils/PageHeader.astro (1)
71-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLimit these link styles to the description.
:global(a)also matches the nested breadcrumb link and overrides its pill styling with an underline. Scope both selectors to.page-description.Proposed fix
- .page-header :global(a) { + .page-description :global(a) { ... } - .page-header :global(a:hover) { + .page-description :global(a:hover) { ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/utils/PageHeader.astro` around lines 71 - 85, Scope the link and hover selectors in the PageHeader styles to `.page-description` so the underline styling applies only to description links. Update both the base `:global(a)` selector and its `:global(a:hover)` counterpart, preserving the breadcrumb link’s existing pill styling.src/components/landing/Hero.astro (2)
252-290:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix transform collision between animation and hover state.
The
.logo-wrapperelement has a continuous CSS animation (floatLogo) that animates thetransformproperty. Because@keyframesanimations take precedence over regular CSS rules for the properties they animate, thetransform: translateY(-4px);in.logo-wrapper:hoverwill be ignored while the animation is running.To fix this seamlessly without altering the DOM, you can use the independent
translateproperty for the hover state, while leaving the@keyframesto animate thetransformproperty. These properties compose perfectly without colliding.[visual_and_interaction]
🐛 Proposed fix using independent transform properties
.logo-wrapper { width: 72px; height: 72px; display: flex; align-items: center; justify-content: center; background: rgba(255, 255, 255, 0.06); border: 1px solid var(--border); border-radius: 16px; backdrop-filter: blur(8px); -webkit-backdrop-filter: blur(8px); box-shadow: 0 8px 32px oklch(0.2 0.02 60 / 0.12), inset 0 1px 0 oklch(1 0 0 / 0.25); animation: floatLogo 6s ease-in-out infinite; transition: - transform 0.25s cubic-bezier(0.34, 1.56, 0.64, 1), + translate 0.25s cubic-bezier(0.34, 1.56, 0.64, 1), box-shadow 0.25s ease, border-color 0.25s ease; } :root.dark .logo-wrapper { background: oklch(0.24 0.008 250); border: 1px solid oklch(0.42 0.01 250); box-shadow: 0 10px 28px oklch(0 0 0 / 0.55), 0 2px 6px oklch(0 0 0 / 0.5), inset 0 1px 0 oklch(1 0 0 / 0.08), inset 0 -1px 0 oklch(0 0 0 / 0.4); } .logo-wrapper:hover { - transform: translateY(-4px); + translate: 0 -4px; box-shadow: 0 16px 42px oklch(0.2 0.02 60 / 0.22), inset 0 1px 0 oklch(1 0 0 / 0.3); border-color: var(--text-muted); }Note: You will also need to update the
prefers-reduced-motionmedia query at line 382 to disabletranslate: none;instead oftransform: none;for the hover state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/landing/Hero.astro` around lines 252 - 290, Update the .logo-wrapper:hover rule to use the independent translate property for the hover lift instead of transform, preserving the floatLogo animation on transform. Also update the prefers-reduced-motion rule to reset translate rather than transform for this hover state.
252-290:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix transform collision between animation and hover state.
The
.logo-wrapperelement has a continuous CSS animation (floatLogo) that animates thetransformproperty. Because@keyframesanimations take precedence over regular CSS rules (including:hoverstates) for the properties they animate, thetransform: translateY(-4px);in.logo-wrapper:hoverwill be ignored while the animation is running.To fix this, either apply the hover transform to an inner element, pause the animation on hover, or use
scale/translateCSS properties independently (if browser support allows).[visual_and_interaction]
🐛 Proposed fix using independent transform properties
Modern CSS allows animating
translateandrotateindependently fromtransform. This avoids the collision..logo-wrapper { - animation: floatLogo 6s ease-in-out infinite; - transition: - transform 0.25s cubic-bezier(0.34, 1.56, 0.64, 1), - box-shadow 0.25s ease, - border-color 0.25s ease; + animation: floatLogo 6s ease-in-out infinite; + transition: + translate 0.25s cubic-bezier(0.34, 1.56, 0.64, 1), + box-shadow 0.25s ease, + border-color 0.25s ease; } .logo-wrapper:hover { - transform: translateY(-4px); + translate: 0 -4px; box-shadow: 0 16px 42px oklch(0.2 0.02 60 / 0.22), inset 0 1px 0 oklch(1 0 0 / 0.3); border-color: var(--text-muted); } `@keyframes` floatLogo { 0%, 100% { - transform: translateY(0px) rotate(0deg); + translate: 0 0; + rotate: 0deg; } 50% { - transform: translateY(-16px) rotate(3deg); + translate: 0 -16px; + rotate: 3deg; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/landing/Hero.astro` around lines 252 - 290, Resolve the transform collision between the continuous floatLogo animation and the hover state on .logo-wrapper. Update the animation and hover styling to use independent translate/scale properties, or otherwise pause/delegate the animation, so hovering visibly applies the intended upward movement while preserving the floating animation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/landing/ReadingSummary.astro`:
- Around line 18-45: Update the latest-book link and each favorites-book link in
ReadingSummary to validate hiveId before interpolating it into the URL. When the
identifier is missing, render the row without a book link or use the existing
profile fallback; preserve the current linked behavior for valid identifiers.
---
Outside diff comments:
In `@src/components/landing/Hero.astro`:
- Around line 252-290: Update the .logo-wrapper:hover rule to use the
independent translate property for the hover lift instead of transform,
preserving the floatLogo animation on transform. Also update the
prefers-reduced-motion rule to reset translate rather than transform for this
hover state.
- Around line 252-290: Resolve the transform collision between the continuous
floatLogo animation and the hover state on .logo-wrapper. Update the animation
and hover styling to use independent translate/scale properties, or otherwise
pause/delegate the animation, so hovering visibly applies the intended upward
movement while preserving the floating animation.
In `@src/components/utils/PageHeader.astro`:
- Around line 71-85: Scope the link and hover selectors in the PageHeader styles
to `.page-description` so the underline styling applies only to description
links. Update both the base `:global(a)` selector and its `:global(a:hover)`
counterpart, preserving the breadcrumb link’s existing pill styling.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d8e1a491-7bc0-4fa0-a72d-8003cb665bbd
📒 Files selected for processing (15)
package.jsonsrc/components/blog/BlogContent.astrosrc/components/blog/BlogSkeleton.astrosrc/components/blog/PostItem.astrosrc/components/events/EventsContent.astrosrc/components/events/EventsSkeleton.astrosrc/components/landing/Hero.astrosrc/components/landing/ReadingSummary.astrosrc/components/utils/PageHeader.astrosrc/layouts/BaseHead.astrosrc/pages/blog.astrosrc/pages/books.astrosrc/pages/events.astrosrc/pages/projects.astrosrc/styles/global.css
💤 Files with no reviewable changes (1)
- src/components/events/EventsContent.astro
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 34 minutes. |
1 participant
Summary by CodeRabbit