feat: add responsive#11
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Redesign footer motto block and contact details (work/personal email, phone, WeChat, website), and tighten responsive alignment across the footer, header, globe section, hero, quote, and subpage hero. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 56 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR applies mobile-first responsive updates across global styles and many components, refactors header/footer navigation and language/select styling, standardizes portrait image layout, converts the activities gallery to a client-side image slider, and refactors the GlobeSection with region presets and a mobile bottom-sheet drag interaction. ChangesMobile Responsive Site Redesign
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Header.tsx (1)
74-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCancel pending RAF in effect cleanup.
The scroll effect removes the event listener but leaves a queued
requestAnimationFramecallback alive. AddcancelAnimationFrame(raf)in cleanup to avoid post-unmount callback execution.Suggested fix
window.addEventListener('scroll', onScroll, { passive: true }); - return () => window.removeEventListener('scroll', onScroll); + return () => { + window.removeEventListener('scroll', onScroll); + if (raf) cancelAnimationFrame(raf); + }; }, []);🤖 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/Header.tsx` around lines 74 - 76, The effect in Header component adds a scroll listener via window.addEventListener('scroll', onScroll) but does not cancel the queued requestAnimationFrame; capture the RAF id (e.g., raf) when calling requestAnimationFrame inside onScroll (or the effect) and in the useEffect cleanup ensure you call cancelAnimationFrame(raf) in addition to window.removeEventListener('scroll', onScroll) so no RAF callback runs after unmount; update the cleanup to reference the same raf variable (useRef or scoped let) used when scheduling the frame.
🤖 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/app/activities/ActivityContent.tsx`:
- Line 19: When the active city/id changes the local slider state "index" can
become stale and cause an out-of-range translation; add a useEffect that watches
the city identifier (id) and/or the images array length and calls setIndex to
clamp the current index into [0, images.length-1] (e.g. setIndex(prev =>
Math.max(0, Math.min(prev, images.length - 1))) or setIndex(0) when you prefer
resetting to the first slide). Place this effect near the existing useState
declaration for index so the component (and the translation logic that uses
index) always gets a valid value after id or images change.
In `@src/components/Footer.tsx`:
- Line 32: The websiteUrl value (const websiteUrl = office?.websiteUrl || '')
can be scheme-less and becomes a relative link; normalize it before binding to
href by trimming whitespace, returning an empty string when absent, and
prefixing "https://" when the value does not start with "http://" or "https://"
(keep existing mailto: or other schemes intact). Update the code that computes
websiteUrl and any places that bind it to href (the websiteUrl variable used in
the Footer component and the other href bindings referenced) so the rendered
href is always an absolute URL or empty.
In `@src/components/Header.tsx`:
- Around line 128-129: The header uses a custom breakpoint `max-[769px]` which
conflicts with the drawer/toggle CSS that targets `max-width: 768px`, causing a
broken edge state; update all occurrences of `max-[769px]` in the Header
component (e.g., the element with className "site-header" and other responsive
className usages inside Header.tsx) to `max-[768px]` so the header layout and
drawer/toggle CSS share the exact same breakpoint.
---
Outside diff comments:
In `@src/components/Header.tsx`:
- Around line 74-76: The effect in Header component adds a scroll listener via
window.addEventListener('scroll', onScroll) but does not cancel the queued
requestAnimationFrame; capture the RAF id (e.g., raf) when calling
requestAnimationFrame inside onScroll (or the effect) and in the useEffect
cleanup ensure you call cancelAnimationFrame(raf) in addition to
window.removeEventListener('scroll', onScroll) so no RAF callback runs after
unmount; update the cleanup to reference the same raf variable (useRef or scoped
let) used when scheduling the frame.
🪄 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: 2b754130-0b65-448d-968a-cd329a3fe362
📒 Files selected for processing (16)
src/app/about/ClosingQuote.tsxsrc/app/about/ContentBlocks.tsxsrc/app/about/TravelRoutes.tsxsrc/app/activities/ActivityContent.tsxsrc/app/contact/ContactContent.tsxsrc/app/globals.csssrc/components/Footer.tsxsrc/components/Header.tsxsrc/components/MottoQuote.tsxsrc/components/SubpageHero.tsxsrc/components/activity/AboutSection.tsxsrc/components/activity/GlobeSection.tsxsrc/components/activity/globe-helpers.tssrc/components/activity/globe-section.csssrc/components/hero/HeroSection.tsxsrc/components/quote/QuoteSection.tsx
| const params = useSearchParams(); | ||
| const id = params?.get('id') ?? ''; | ||
| const city = cities.find((c) => c.slug === id); | ||
| const [index, setIndex] = useState(0); |
There was a problem hiding this comment.
Reset/clamp slider index when city changes.
Line 19 keeps stale index across id changes; when the next city has fewer images, Line 61 can translate past the last slide and show an empty frame.
Suggested fix
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
@@
const photos = city.photos ?? [];
const last = photos.length - 1;
+
+ useEffect(() => {
+ setIndex(0);
+ }, [city.slug]);Also applies to: 37-39, 61-61
🤖 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/app/activities/ActivityContent.tsx` at line 19, When the active city/id
changes the local slider state "index" can become stale and cause an
out-of-range translation; add a useEffect that watches the city identifier (id)
and/or the images array length and calls setIndex to clamp the current index
into [0, images.length-1] (e.g. setIndex(prev => Math.max(0, Math.min(prev,
images.length - 1))) or setIndex(0) when you prefer resetting to the first
slide). Place this effect near the existing useState declaration for index so
the component (and the translation logic that uses index) always gets a valid
value after id or images change.
| const addressLines = office?.addressLines && office.addressLines.length > 0 | ||
| ? office.addressLines | ||
| : [t('footer.location')]; | ||
| const websiteUrl = office?.websiteUrl || ''; |
There was a problem hiding this comment.
Normalize websiteUrl before binding to href.
Right now a scheme-less value (e.g., tabernam.com) becomes a relative link. Normalize to absolute https://... before rendering.
Suggested fix
- const websiteUrl = office?.websiteUrl || '';
+ const websiteUrlRaw = (office?.websiteUrl || '').trim();
+ const websiteUrl = websiteUrlRaw
+ ? (/^https?:\/\//i.test(websiteUrlRaw) ? websiteUrlRaw : `https://${websiteUrlRaw}`)
+ : '';Also applies to: 109-113
🤖 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/Footer.tsx` at line 32, The websiteUrl value (const websiteUrl
= office?.websiteUrl || '') can be scheme-less and becomes a relative link;
normalize it before binding to href by trimming whitespace, returning an empty
string when absent, and prefixing "https://" when the value does not start with
"http://" or "https://" (keep existing mailto: or other schemes intact). Update
the code that computes websiteUrl and any places that bind it to href (the
websiteUrl variable used in the Footer component and the other href bindings
referenced) so the rendered href is always an absolute URL or empty.
| className="site-header fixed top-0 left-0 right-0 z-100 bg-gray-20 grid grid-cols-[auto_1fr_auto] max-[769px]:grid-cols-[1fr_auto] items-center gap-6 px-[var(--side-padding)] max-md:px-[16px] py-2.5" | ||
| > |
There was a problem hiding this comment.
Unify mobile breakpoint contracts (769 vs 768) to avoid a broken edge state.
Line 128/138/146/152/158 use max-[769px], while the drawer/toggle CSS in src/app/globals.css only activates at max-width: 768px. At exactly 769px, header layout switches to mobile classes but drawer mechanics/toggle CSS don’t, causing inconsistent nav behavior.
Suggested fix
- className="... max-[769px]:grid-cols-[1fr_auto] ..."
+ className="... max-[768px]:grid-cols-[1fr_auto] ..."
- className="nav ... max-[769px]:gap-0"
+ className="nav ... max-[768px]:gap-0"
- <div className="hidden max-[769px]:block">
+ <div className="hidden max-[768px]:block">
- <div className="max-[769px]:hidden">
+ <div className="max-[768px]:hidden">
- className="nav-toggle ... hidden max-[769px]:block"
+ className="nav-toggle ... hidden max-[768px]:block"Also applies to: 138-138, 146-158
🤖 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/Header.tsx` around lines 128 - 129, The header uses a custom
breakpoint `max-[769px]` which conflicts with the drawer/toggle CSS that targets
`max-width: 768px`, causing a broken edge state; update all occurrences of
`max-[769px]` in the Header component (e.g., the element with className
"site-header" and other responsive className usages inside Header.tsx) to
`max-[768px]` so the header layout and drawer/toggle CSS share the exact same
breakpoint.
Add responsive idle zoom (idleZoomFor) and restore-on-close camera handling for city cards in the globe section, plus assorted responsive adjustments across header, hero, quote, activities, about, and UI components. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 (2)
src/components/activity/GlobeSection.tsx (2)
289-318:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestored cards can close back to the wrong viewport.
When
activeIdxis rehydrated from the saved globe state,prevActiveIdxRef.currentis stillnull, so this branch snapshots the idle camera before anyregionKeyframing has happened. The close-button path then reuses that stale camera and the map comes back at the idle/world view while the restored region tab stays selected. Persist the pre-city camera in the restore payload, or bypassrestoreOnCloseReffor restored sessions so close falls through to the regionfitBounds.🤖 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/activity/GlobeSection.tsx` around lines 289 - 318, The restore path is capturing a stale preCityCamera because prevActiveIdxRef.current is null during rehydration; update the logic around prevActiveIdxRef, preCityCameraRef, and restoreOnCloseRef so restored sessions don't snapshot or use that stale camera: either persist preCityCameraRef into the saved restore payload when serializing the globe state, or (simpler) skip setting preCityCameraRef and set restoreOnCloseRef.current = false when activeIdx was rehydrated so the close-button branch falls through to the region fitBounds logic; specifically change the block that currently checks prevActiveIdxRef.current === null (and the subsequent map.flyTo) to only set preCityCameraRef when prevActiveIdxRef.current !== null or when a new non‑rehydrated activation occurs, and ensure restoreOnCloseRef is cleared for rehydrated activeIdx cases.
122-129:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe min-zoom toast misses the real floor after
maxBounds.
maxBoundsmakes the effective minimum zoom viewport-dependent, but this listener still only compares againstMIN_ZOOM. On larger viewports the map clamps above1.2, so wheel/pinch users can hit the real floor without ever seeing the “minimum zoom” feedback. Reuse the same floor calculation fromzoomBy()here so gesture and button zoom paths stay consistent.Also applies to: 151-160
🤖 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/activity/GlobeSection.tsx` around lines 122 - 129, The min-zoom toast is checking only MIN_ZOOM but the effective minimum zoom is clamped by maxBounds (viewport-dependent), so update the zoom-change/gesture listener(s) that currently compare against MIN_ZOOM (the block around renderWorldCopies/maxBounds and the similar block later) to reuse the same floor calculation used in zoomBy(): compute the effective minimum zoom the same way zoomBy() does and compare against that value, then show the “minimum zoom” toast when the gesture hits that computed floor; reference MIN_ZOOM, maxBounds, and the zoomBy() helper so you mirror its exact logic rather than using a hardcoded MIN_ZOOM check.
♻️ Duplicate comments (1)
src/app/activities/ActivityContent.tsx (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset slider index when city changes.
The past review comment remains valid. When the user navigates to a different city (via query param
id), theindexstate is not reset. If the previous city hadindex=3and the new city has only 2 photos, line 61 will computetranslateX(-300%)on a 2-slide track, displaying a blank frame.🔄 Proposed fix
-import { useState } from 'react'; +import { useEffect, useState } from 'react';Then add an effect after line 19:
const [index, setIndex] = useState(0); + +useEffect(() => { + setIndex(0); +}, [id]);Also applies to: 19-19
🤖 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/app/activities/ActivityContent.tsx` at line 3, The slider index isn't reset when the route/query `id` (city) changes, so add a useEffect in the ActivityContent component that watches the city identifier (the `id` query param or prop used to fetch `photos`) and calls setIndex(0) when it changes; ensure the effect depends on that id (and optionally on `photos.length`) to reset `index` to 0 to avoid computing an out-of-range translateX for the new photo set.
🧹 Nitpick comments (3)
src/app/activities/ActivityContent.tsx (3)
68-69: ⚖️ Poor tradeoffConsider using
next/imagefor optimization.The slider uses a regular
<img>tag, which bypasses Next.js automatic image optimization, lazy loading, and responsive srcsets. While Directus CMS URLs may require additional configuration, integratingnext/imagewould improve performance and user experience.Example migration
-{/* eslint-disable-next-line `@next/next/no-img-element` */} -<img className="w-full h-full object-cover" src={src} alt={`${name} — ${i + 1}`} /> +<Image + className="object-cover" + src={src} + alt={`${name} — ${i + 1}`} + fill + sizes="(max-width: 768px) 100vw, 1320px" +/>You'll need to import
Imagefromnext/imageand configureremotePatternsinnext.config.jsfor your Directus domain.🤖 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/app/activities/ActivityContent.tsx` around lines 68 - 69, Replace the plain <img> in ActivityContent (the element rendering src/alt using name and i) with Next.js Image: import Image from "next/image", remove the eslint-disable comment, and render Image with appropriate props (either width/height or fill plus object-fit via className) and use alt={`${name} — ${i + 1}`} and src={src}; additionally, add your Directus domain to next.config.js remotePatterns so external Directus URLs are allowed by next/image. Ensure you update any surrounding markup/CSS to support Image's layout (e.g., position: relative when using fill).
97-107: 💤 Low valueOptionally add
aria-liveregion for screen reader announcements.The dot navigation allows users to jump between slides, but screen reader users won't receive automatic announcements of slide changes. Consider adding an
aria-live="polite"region to announce the current slide.Example implementation
<div className="mt-6 flex justify-center gap-2"> + <div className="sr-only" aria-live="polite" aria-atomic="true"> + Image {index + 1} of {photos.length} + </div> {photos.map((_, i) => (Add this CSS if
sr-onlyis not defined:.sr-only { position: absolute; width: 1px; height: 1px; padding: 0; margin: -1px; overflow: hidden; clip: rect(0, 0, 0, 0); white-space: nowrap; border-width: 0; }🤖 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/app/activities/ActivityContent.tsx` around lines 97 - 107, Add an aria-live region next to the dot navigation so screen readers announce slide changes: create a visually-hidden element (e.g., class "sr-only") within the ActivityContent component near the div that renders photos.map(...) and the buttons, and render text like `Current slide ${index + 1} of ${photos.length}` inside it; update this element by referencing the existing index state (used by setIndex) so it changes whenever a user clicks a dot. Ensure the element uses aria-live="polite" and is styled with an sr-only utility (or add the provided sr-only CSS) so it is announced but not visible.
104-104: ActivityContent.tsx:bg-gray-60is defined in the theme
src/app/globals.cssdefines the theme token--color-gray-60(backed by--gray-60:#C2CBD7``), sobg-gray-60is a valid palette utility.- Optional: consider using
next/imagefor the slider images (if it fits your Directus setup) and add anaria-liveannouncement when the slide changes.🤖 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/app/activities/ActivityContent.tsx` at line 104, The class bg-gray-60 used in ActivityContent.tsx is valid because globals.css defines --color-gray-60, so either leave the current class on the element (the template string containing i === index ? 'w-6 bg-brand' : 'w-2 bg-gray-60') or, to avoid Tailwind purge/utility-missing risks, replace the utility with an explicit CSS-variable utility such as bg-[color:var(--color-gray-60)] or ensure your Tailwind config safelists 'bg-gray-60'; verify globals.css contains --color-gray-60 and that your build config will preserve that utility.
🤖 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/LangSwitcher.tsx`:
- Line 16: The Tailwind v3-style leading "!" inside arbitrary variants in the
LangSwitcher className (e.g., "[&>button]:!text-white",
"[&>button]:hover:!bg-transparent", "[&>button>svg]:!text-white") must be
changed to the v4 trailing suffix form (e.g., "[&>button]:text-white!",
"[&>button]:hover:bg-transparent!", "[&>button>svg]:text-white!") so the
important overrides apply; update the className string in the LangSwitcher
component accordingly and then re-check text contrast at both header breakpoints
where LangSwitcher is rendered to ensure accessibility.
---
Outside diff comments:
In `@src/components/activity/GlobeSection.tsx`:
- Around line 289-318: The restore path is capturing a stale preCityCamera
because prevActiveIdxRef.current is null during rehydration; update the logic
around prevActiveIdxRef, preCityCameraRef, and restoreOnCloseRef so restored
sessions don't snapshot or use that stale camera: either persist
preCityCameraRef into the saved restore payload when serializing the globe
state, or (simpler) skip setting preCityCameraRef and set
restoreOnCloseRef.current = false when activeIdx was rehydrated so the
close-button branch falls through to the region fitBounds logic; specifically
change the block that currently checks prevActiveIdxRef.current === null (and
the subsequent map.flyTo) to only set preCityCameraRef when
prevActiveIdxRef.current !== null or when a new non‑rehydrated activation
occurs, and ensure restoreOnCloseRef is cleared for rehydrated activeIdx cases.
- Around line 122-129: The min-zoom toast is checking only MIN_ZOOM but the
effective minimum zoom is clamped by maxBounds (viewport-dependent), so update
the zoom-change/gesture listener(s) that currently compare against MIN_ZOOM (the
block around renderWorldCopies/maxBounds and the similar block later) to reuse
the same floor calculation used in zoomBy(): compute the effective minimum zoom
the same way zoomBy() does and compare against that value, then show the
“minimum zoom” toast when the gesture hits that computed floor; reference
MIN_ZOOM, maxBounds, and the zoomBy() helper so you mirror its exact logic
rather than using a hardcoded MIN_ZOOM check.
---
Duplicate comments:
In `@src/app/activities/ActivityContent.tsx`:
- Line 3: The slider index isn't reset when the route/query `id` (city) changes,
so add a useEffect in the ActivityContent component that watches the city
identifier (the `id` query param or prop used to fetch `photos`) and calls
setIndex(0) when it changes; ensure the effect depends on that id (and
optionally on `photos.length`) to reset `index` to 0 to avoid computing an
out-of-range translateX for the new photo set.
---
Nitpick comments:
In `@src/app/activities/ActivityContent.tsx`:
- Around line 68-69: Replace the plain <img> in ActivityContent (the element
rendering src/alt using name and i) with Next.js Image: import Image from
"next/image", remove the eslint-disable comment, and render Image with
appropriate props (either width/height or fill plus object-fit via className)
and use alt={`${name} — ${i + 1}`} and src={src}; additionally, add your
Directus domain to next.config.js remotePatterns so external Directus URLs are
allowed by next/image. Ensure you update any surrounding markup/CSS to support
Image's layout (e.g., position: relative when using fill).
- Around line 97-107: Add an aria-live region next to the dot navigation so
screen readers announce slide changes: create a visually-hidden element (e.g.,
class "sr-only") within the ActivityContent component near the div that renders
photos.map(...) and the buttons, and render text like `Current slide ${index +
1} of ${photos.length}` inside it; update this element by referencing the
existing index state (used by setIndex) so it changes whenever a user clicks a
dot. Ensure the element uses aria-live="polite" and is styled with an sr-only
utility (or add the provided sr-only CSS) so it is announced but not visible.
- Line 104: The class bg-gray-60 used in ActivityContent.tsx is valid because
globals.css defines --color-gray-60, so either leave the current class on the
element (the template string containing i === index ? 'w-6 bg-brand' : 'w-2
bg-gray-60') or, to avoid Tailwind purge/utility-missing risks, replace the
utility with an explicit CSS-variable utility such as
bg-[color:var(--color-gray-60)] or ensure your Tailwind config safelists
'bg-gray-60'; verify globals.css contains --color-gray-60 and that your build
config will preserve that utility.
🪄 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: 5088d387-5f2d-4eb8-a243-271ef6aaecac
📒 Files selected for processing (15)
src/app/about/HeroHeader.tsxsrc/app/about/TravelRoutes.tsxsrc/app/activities/ActivityContent.tsxsrc/app/globals.csssrc/components/Header.tsxsrc/components/HomeMarquee.tsxsrc/components/LangSwitcher.tsxsrc/components/SubpageHero.tsxsrc/components/activity/AboutSection.tsxsrc/components/activity/GlobeSection.tsxsrc/components/activity/globe-helpers.tssrc/components/activity/globe-section.csssrc/components/hero/HeroSection.tsxsrc/components/quote/QuoteSection.tsxsrc/components/ui/Select.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/HomeMarquee.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/SubpageHero.tsx
- src/components/quote/QuoteSection.tsx
- src/components/activity/globe-helpers.ts
- src/components/Header.tsx
- src/app/globals.css
- src/components/activity/globe-section.css
| onChange={switchLang} | ||
| ariaLabel="Language" | ||
| className="lang-switcher ml-1 [&>button]:!text-dark [&>button]:!text-[18px] [&>button]:!font-normal [&>button]:!normal-case [&>button]:!tracking-[-0.007em] [&>button]:hover:!bg-transparent [&>button>svg]:!text-dark" | ||
| className="lang-switcher ml-1 [&>button]:!text-white [&>button]:!text-[18px] [&>button]:!font-normal [&>button]:!normal-case [&>button]:!tracking-[-0.007em] [&>button]:hover:!bg-transparent [&>button>svg]:!text-white" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all Tailwind important modifiers using old v3 syntax.
# Expect: Instances of `:!` or `!text-` or other `!utility-` patterns.
rg -n ':\!' --type=tsx --type=ts --type=jsx --type=js -C2Repository: 8seneca-hub/tabernam-site
Length of output: 140
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect Tailwind version
echo "== package.json (tailwind-related) =="
if [ -f package.json ]; then
cat package.json | sed -n '1,200p' | rg -n '"tailwindcss"|tailwindcss|tailwind' || true
else
echo "package.json not found"
fi
# 2) Inspect LangSwitcher className line(s)
echo
echo "== src/components/LangSwitcher.tsx (relevant lines) =="
if [ -f src/components/LangSwitcher.tsx ]; then
nl -ba src/components/LangSwitcher.tsx | sed -n '1,120p' | rg -n 'LangSwitcher|lang-switcher|className' -n || true
# Print around line ~16 regardless
nl -ba src/components/LangSwitcher.tsx | sed -n '10,40p'
else
echo "src/components/LangSwitcher.tsx not found"
fi
# 3) Find Tailwind v3-style important-in-arbitrary-variant syntax: :!...
echo
echo "== Occurrences of ':!' in TS/TSX/JS/JSX (arbitrary variants) =="
rg -n ':!' --type=tsx --type=ts --type=jsx --type=js -C1 || true
# Also catch plain '!text-' patterns in class strings
echo
echo "== Occurrences of '!text-' / '!bg-' / '!font-' in TS/TSX/JS/JSX =="
rg -n "![a-zA-Z]+-(?:\\[.*?\\]|[-_a-zA-Z0-9]+)" --type=tsx --type=ts --type=jsx --type=js -C0 || true
# 4) Find where LangSwitcher is rendered so we can reason about backgrounds/contrast
echo
echo "== Usages of <LangSwitcher .../> =="
rg -n '<\s*LangSwitcher\b' -S src --glob='*.tsx' --glob='*.ts' || true
echo
echo "== Usages of LangSwitcher symbol =="
rg -n 'LangSwitcher' -S src --glob='*.tsx' --glob='*.ts' -C1 || trueRepository: 8seneca-hub/tabernam-site
Length of output: 339
🌐 Web query:
Tailwind CSS v4 important modifier arbitrary variants syntax order ! must come after utility name text-white! :!text-white breaking change
💡 Result:
In Tailwind CSS v4, the important modifier syntax has been updated to improve consistency and readability. The recommended way to mark a utility as important is to append the exclamation mark ! to the very end of the utility class (e.g., text-white!) [1][2][3]. While the previous v3 syntax (placing ! at the beginning of the utility name, such as !text-white) is still supported for backwards compatibility, it is considered deprecated [4][3]. Key details regarding this change: 1. Consistency: Moving the ! to the end makes the syntax easier to reason about, particularly when using multiple variants (e.g., hover:text-white!) [5]. 2. Arbitrary Variants: The rule applies consistently to standard utilities and arbitrary variants [1]. When using arbitrary variants, the important modifier should still be applied to the specific utility class being modified [3]. 3. Migration: The Tailwind CSS upgrade tool (npx @tailwindcss/upgrade``) is designed to handle this migration automatically [6]. 4. @apply usage: Inside `@apply` directives, the `!important` suffix is the standard approach in v4; placing `!` at the start of utilities within `@apply` is deprecated [5]. For projects that require all utility classes to be marked as important, you can use the `important` flag during import, which replaces the `important: true` configuration in the older `tailwind.config.js` format [7][8]: `@import` "tailwindcss" important; Top results for this research: [4], [5], [7], [1], [2], [3], [8], [6]
Citations:
- 1: https://josedacosta.github.io/tailwindcss-obfuscator/research/tailwind-v4-summary
- 2: https://combray.prose.sh/2025-11-30-tailwind-best-practices
- 3: https://tailwindcss.com/docs/upgrade-guide
- 4: [v4] Support old important modifier position tailwindlabs/tailwindcss#13103
- 5: Add initial codemod tooling tailwindlabs/tailwindcss#14434
- 6: https://dev.to/dataformathub/tailwind-css-v4-deep-dive-why-the-oxide-engine-changes-everything-in-2026-2595
- 7: Add support for
importantin v4 tailwindlabs/tailwindcss#14448 - 8: https://tailwindcss.com/docs/styling-with-utility-classes
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show LangSwitcher.tsx with line numbers around the target area
echo "== File: src/components/LangSwitcher.tsx =="
if [ -f src/components/LangSwitcher.tsx ]; then
awk 'NR>=1 && NR<=120 {printf "%5d:%s\n", NR, $0}' src/components/LangSwitcher.tsx
else
echo "Missing: src/components/LangSwitcher.tsx"
fi
# 2) Find usages of LangSwitcher
echo
echo "== Usages of <LangSwitcher .../> =="
rg -n "<\\s*LangSwitcher\\b" src --glob='*.tsx' --glob='*.ts' || true
echo
echo "== Usages of LangSwitcher identifier =="
rg -n "\\bLangSwitcher\\b" src --glob='*.tsx' --glob='*.ts' -C1 || true
# 3) Find old arbitrary-variant important form: :!
echo
echo "== Occurrences of ':!' in class strings / TS/TSX/JS/JSX =="
rg -n ":\!" src --type=tsx --type=ts --type=jsx --type=js -C1 || true
# 4) Find v4-style important suffix in class strings
echo
echo "== Occurrences of '*!' important suffix in class strings (common ones) =="
rg -n "text-white!" src --type=tsx --type=ts --type=jsx --type=js -C0 || true
rg -n "\\!bg-" src --type=tsx --type=ts --type=jsx --type=js -C0 || true
rg -n "[a-zA-Z0-9_\\-\\[\\]]+!" src --type=tsx --type=ts --type=jsx --type=js -C0 || trueRepository: 8seneca-hub/tabernam-site
Length of output: 2225
🌐 Web query:
Tailwind CSS v4 arbitrary variants important modifier syntax example "[&>button]:text-white!" and whether "[&>button]:!text-white" works
💡 Result:
In Tailwind CSS v4, the syntax for the important modifier has been standardized to be used as a suffix at the end of a utility class [1][2]. For your example, the correct syntax is: [&>button]:text-white! This applies the!important modifier to the text-white utility within the arbitrary variant [3]. Regarding your question about [&>button]:!text-white: This syntax is generally not supported in Tailwind CSS v4 [1][2]. Placing the! modifier at the beginning of the utility class (inside the arbitrary variant or otherwise) was the convention in v3, but v4 strictly requires the modifier to be placed as a suffix [1][2]. While some build configurations or legacy setups might occasionally process the prefix correctly depending on how the utility is parsed, the official and recommended practice for v4 is to use the trailing! suffix for all utility classes [1][2].
Citations:
- 1: https://github.com/dxos/dxos/blob/e4919fd46f87a8107cac7af0def56ea6136edc1f/TAILWIND_V4_UTILITY_CHANGES.md
- 2: https://josedacosta.github.io/tailwindcss-obfuscator/research/tailwind-v4-features-analysis
- 3: https://tailwindcss.com/docs/styling-with-utility-classes
🌐 Web query:
Does Tailwind CSS v4 support the old important prefix syntax inside arbitrary variants like "[&>button]:!text-white" or is it invalid (must be "[&>button]:text-white!")?
💡 Result:
In Tailwind CSS v4, the correct syntax for making a utility important is to place the! modifier at the end of the class name (e.g., text-white!) [1]. While Tailwind CSS v4 generally maintains some backwards compatibility for v3 features, the established and recommended syntax for marking utilities as important in v4 is exclusively the trailing! modifier [1]. Regarding arbitrary variants like [&>button]:!text-white, this syntax is effectively invalid in the context of v4's preferred styling patterns. You should use the trailing modifier syntax within the arbitrary variant: [&>button]:text-white! [1]. To summarize the relevant changes in v4: - The! modifier must be placed at the end of the class name, whereas in v3 it was placed at the beginning [1]. - This trailing! syntax applies to all utilities, including those used within arbitrary variants [1]. - While some older patterns may be supported for transitionary compatibility, the v4 documentation explicitly instructs developers to adopt the trailing! modifier [1].
Citations:
Fix Tailwind v4 !important syntax in LangSwitcher className.
src/components/LangSwitcher.tsx uses v3-style leading ! inside arbitrary variants (e.g., [&>button]:!text-white, [&>button]:hover:!bg-transparent, [&>button>svg]:!text-white). Tailwind v4 expects the trailing suffix form (text-white!, bg-transparent!), otherwise these important overrides may not apply.
🐛 Proposed fix
- className="lang-switcher ml-1 [&>button]:!text-white [&>button]:!text-[18px] [&>button]:!font-normal [&>button]:!normal-case [&>button]:!tracking-[-0.007em] [&>button]:hover:!bg-transparent [&>button>svg]:!text-white"
+ className="lang-switcher ml-1 [&>button]:text-white! [&>button]:text-[18px]! [&>button]:font-normal! [&>button]:normal-case! [&>button]:tracking-[-0.007em]! [&>button]:hover:bg-transparent! [&>button>svg]:text-white!"After updating, re-check text contrast in both header breakpoints where LangSwitcher is rendered.
📝 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.
| className="lang-switcher ml-1 [&>button]:!text-white [&>button]:!text-[18px] [&>button]:!font-normal [&>button]:!normal-case [&>button]:!tracking-[-0.007em] [&>button]:hover:!bg-transparent [&>button>svg]:!text-white" | |
| className="lang-switcher ml-1 [&>button]:text-white! [&>button]:text-[18px]! [&>button]:font-normal! [&>button]:normal-case! [&>button]:tracking-[-0.007em]! [&>button]:hover:bg-transparent! [&>button>svg]:text-white!" |
🤖 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/LangSwitcher.tsx` at line 16, The Tailwind v3-style leading
"!" inside arbitrary variants in the LangSwitcher className (e.g.,
"[&>button]:!text-white", "[&>button]:hover:!bg-transparent",
"[&>button>svg]:!text-white") must be changed to the v4 trailing suffix form
(e.g., "[&>button]:text-white!", "[&>button]:hover:bg-transparent!",
"[&>button>svg]:text-white!") so the important overrides apply; update the
className string in the LangSwitcher component accordingly and then re-check
text contrast at both header breakpoints where LangSwitcher is rendered to
ensure accessibility.
1 participant
Summary by CodeRabbit
New Features
Style
Bug Fixes