⚡ Bolt: Fix hydration mismatch in Countdown component#49
Conversation
Initializes the Countdown state with null instead of Date.now() to ensure the initial server render matches the client hydration. The time difference is then calculated immediately on mount using useEffect, preventing hydration errors and ensuring the countdown starts correctly on the client. Also improved interval logic to clamp to zero and clear interval when finished. Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThis PR introduces comprehensive project governance, quality infrastructure, and architectural improvements. Changes include new documentation (ARCHITECTURE.md, SECURITY.md, CONTRIBUTING.md), refactored PWA/service worker configuration, expanded testing capabilities (mutation, fuzz, snapshot tests), consolidated navigation configuration with conditional rendering, TypeScript immutability constraints on components, CI/CD workflow enhancements, and removal of deprecated files. The changes span governance, testing, PWA handling, navigation logic, and code quality. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 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 |
Summary of ChangesHello @anyulled, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical hydration mismatch issue within the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves the hydration mismatch issue in the Countdown component by deferring the time calculation to the client side. The refactoring to recalculate the time difference on each interval tick also improves the countdown's accuracy. I've added one suggestion to optimize the performance within the useEffect hook by avoiding repeated date parsing.
…ess test Initializes the Countdown state with null instead of Date.now() to ensure the initial server render matches the client hydration. The time difference is then calculated immediately on mount using useEffect, preventing hydration errors and ensuring the countdown starts correctly on the client. Also improved interval logic to clamp to zero and clear interval when finished. Additionally, updated `cypress/e2e/home/home-editions.cy.ts` to remove an outdated assertion for `h5` tags, which was causing CI failures as the component now uses `div` and `span` elements. Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
…ess test Initializes the Countdown state with null instead of Date.now() to ensure the initial server render matches the client hydration. The time difference is then calculated immediately on mount using useEffect, preventing hydration errors and ensuring the countdown starts correctly on the client. Also improved interval logic to clamp to zero and clear interval when finished. Further optimized to cache the parsed target event date to avoid repeated Date object creation on each interval tick. Additionally, updated `cypress/e2e/home/home-editions.cy.ts` to remove an outdated assertion for `h5` tags, which was causing CI failures as the component now uses `div` and `span` elements. Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
This pr is 41 commits behind. @jules please address that |
I've rebased the branch onto the latest |
…ess test Initializes the Countdown state with null instead of Date.now() to ensure the initial server render matches the client hydration. The time difference is then calculated immediately on mount using useEffect, preventing hydration errors and ensuring the countdown starts correctly on the client. Also improved interval logic to clamp to zero and clear interval when finished. Further optimized to cache the parsed target event date to avoid repeated Date object creation on each interval tick. Additionally, updated `cypress/e2e/home/home-editions.cy.ts` to remove an outdated assertion for `h5` tags, which was causing CI failures as the component now uses `div` and `span` elements. Rebased the branch on top of `main` to address the PR being behind by 41 commits, and resolved all conflicts, fixing `fast-check` type errors and test issues in the process. Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
styles/layout/pages/_others.scss (1)
1697-1721: 🛠️ Refactor suggestion | 🟠 MajorRemove duplicate
.sponsor-carddefinition.This
.sponsor-cardblock is redundant. Line 3 imports_brands.scssvia@import "./brands";, which already contains an identical.sponsor-carddefinition (lines 394-418 in_brands.scss). Having duplicate definitions increases maintenance burden and can lead to inconsistencies if one is updated but not the other.♻️ Proposed fix: Remove the duplicate block
-// Sponsor card container -.sponsor-card { - display: flex; - align-items: center; - justify-content: center; - padding: 1rem; - transition: transform 0.3s ease; - background-color: rgb(255 255 255 / 0.9); - backdrop-filter: blur(10px); - border: 1px solid rgb(255 255 255 / 0.1); - border-radius: 8px; - height: 100%; - min-height: 100px; - - &:hover { - transform: translateY(-5px); - } - - img { - max-height: 100px; - width: auto; - max-width: 100%; - object-fit: contain; - margin: 0 auto; - } -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/layout/pages/_others.scss` around lines 1697 - 1721, The .sponsor-card block in this file duplicates the definition already provided by the imported stylesheet (`@import` "./brands"); remove this redundant .sponsor-card rule from styles/layout/pages/_others.scss so the project uses the single canonical definition in _brands.scss, ensuring you only delete the entire .sponsor-card rule (including its nested img and &:hover rules) and leave the `@import` "./brands"; line intact.components/layout/Popup.tsx (1)
33-40:⚠️ Potential issue | 🔴 CriticalEvent listeners are never removed — memory leak.
The cleanup function passes new anonymous functions
() => {}toremoveEventListener, butremoveEventListenerrequires the exact same function reference that was passed toaddEventListener. The original handlers from lines 18 and 26 remain attached, causing memory leaks on component unmount/remount.🐛 Proposed fix: store handler references
useEffect(() => { const popup = document.getElementById("popup"); const closeBtn = document.getElementById("close-popup"); const noThanksBtn = document.querySelector(".no-thanks"); + const hidePopup = () => { + if (popup) { + popup.style.display = "none"; + } + }; + if (popup) { setTimeout(() => { popup.style.display = "flex"; }, 100); } if (closeBtn) { - closeBtn.addEventListener("click", () => { - if (popup) { - popup.style.display = "none"; - } - }); + closeBtn.addEventListener("click", hidePopup); } if (noThanksBtn) { - noThanksBtn.addEventListener("click", () => { - if (popup) { - popup.style.display = "none"; - } - }); + noThanksBtn.addEventListener("click", hidePopup); } return () => { if (closeBtn) { - closeBtn.removeEventListener("click", () => {}); + closeBtn.removeEventListener("click", hidePopup); } if (noThanksBtn) { - noThanksBtn.removeEventListener("click", () => {}); + noThanksBtn.removeEventListener("click", hidePopup); } }; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/layout/Popup.tsx` around lines 33 - 40, The cleanup currently passes new anonymous functions to removeEventListener so the original click handlers (the inline arrow functions added to closeBtn and noThanksBtn) are never removed; fix this by extracting those inline handlers into stable references (e.g., const handleCloseClick = (...) => { ... } and const handleNoThanksClick = (...) => { ... }), use those variables when calling addEventListener on closeBtn and noThanksBtn, and then call removeEventListener with the same handleCloseClick and handleNoThanksClick in the return cleanup to properly detach the listeners.components/layout/header/Header8.tsx (1)
9-16:⚠️ Potential issue | 🟡 MinorRemoving these unused props requires updating the parent component.
isSearchandhandleSearchare defined inHeaderPropsbut not used in the Header8 component. However, they are actively passed fromDynamicHeaderWrapper.tsx(<Header8 scroll={scroll} isSearch={isSearch} handleSearch={handleSearch} navigation={navigation} />). Removing them from the interface will cause a type error there. Update both the interface definition and the calling code inDynamicHeaderWrapper.tsxto remove these props.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/layout/header/Header8.tsx` around lines 9 - 16, HeaderProps declares isSearch and handleSearch but Header8 does not use them; remove those two props from the HeaderProps interface and from the Header8 parameter type so HeaderProps only contains scroll and navigation (and update the function signature export default function Header8({ scroll, navigation }: Readonly<HeaderProps>) accordingly). Then update the caller in DynamicHeaderWrapper.tsx to stop passing isSearch and handleSearch to <Header8 ... /> (leave scroll and navigation as before) and remove any related imports/usages there; also search for other callers of Header8 and update them similarly.
🧹 Nitpick comments (12)
styles/layout/pages/_brands.scss (1)
394-418: Duplicate.sponsor-carddefinition exists in_others.scss.This exact
.sponsor-cardblock (including the same property updates) is duplicated in_others.scss(lines 1697-1721). Since_others.scssimports_brands.scssvia@import "./brands";, one of these definitions should be removed to avoid redundancy and potential maintenance issues.Consider keeping the definition in only one file (likely
_brands.scssgiven it's in the sponsors CSS area section) and removing the duplicate from_others.scss.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/layout/pages/_brands.scss` around lines 394 - 418, Duplicate .sponsor-card rules are defined in both _brands.scss and _others.scss; remove the redundant block from one file to avoid duplication and maintenance drift. Keep the authoritative definition (preferably the one in _brands.scss) and delete the matching .sponsor-card selector block in _others.scss (including its &:hover and img nested rules), or alternatively remove the one in _brands.scss and ensure _others.scss still imports "./brands"; update any imports if you move the definition so only a single .sponsor-card selector remains in the codebase.styles/layout/pages/_others.scss (1)
1683-1694: These sponsor-related classes are also duplicated.The
.sponsors-section-headingand.sponsor-category-titleclasses (lines 1683-1694) are also duplicated from_brands.scss(lines 380-391). Consider removing these as well for consistency.♻️ Proposed fix: Remove the duplicate classes
-// Sponsors section heading -.sponsors-section-heading { - color: white !important; -} - -// Sponsor category title -.sponsor-category-title { - color: white !important; - opacity: 0.7; - text-transform: uppercase; - letter-spacing: 2px; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/layout/pages/_others.scss` around lines 1683 - 1694, Remove the duplicated CSS rules for .sponsors-section-heading and .sponsor-category-title from this file (_others.scss) and keep the single source of truth in the existing _brands.scss; locate the two class blocks shown and delete them, then run a quick grep/IDE search for .sponsors-section-heading and .sponsor-category-title to confirm no other duplicates or missing references remain and rebuild to verify styles are unchanged.components/layout/Popup.tsx (1)
6-41: Consider refactoring to React state/refs pattern.Direct DOM manipulation via
getElementById/querySelectorand manualstyle.displaytoggling bypasses React's declarative model. This makes the component harder to test and reason about.A more idiomatic approach would use
useStatefor visibility and React's event handling:♻️ Idiomatic React approach
export default function Popup() { const [isVisible, setIsVisible] = useState(false); useEffect(() => { const timer = setTimeout(() => setIsVisible(true), 100); return () => clearTimeout(timer); }, []); if (!isVisible) return null; return ( <div id="popup" className="popup-overlay"> <div className="popup-content"> <span className="close-btn" id="close-popup" onClick={() => setIsVisible(false)} > × </span> {/* ... rest of content ... */} <p className="no-thanks" onClick={() => setIsVisible(false)}> No thanks </p> </div> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/layout/Popup.tsx` around lines 6 - 41, Replace the manual DOM manipulation in Popup (the useEffect that uses getElementById/querySelector, closeBtn/noThanksBtn and style.display toggles) with React state and handlers: add a local state like isVisible and setIsVisible, use useEffect only to start/clear a timer that sets isVisible true (clearTimeout on cleanup), remove addEventListener/removeEventListener logic, and wire the close action to onClick handlers on the close element and the "no-thanks" element in the Popup component JSX so visibility is controlled declaratively by isVisible.styles/layout/footer/_footer-1.scss (1)
1776-1783: Consider:aspect-ratiomay be redundant with explicit dimensions.With both
width: 100%andheight: 250pxalready set, theaspect-ratioproperty will effectively be ignored by the browser since both dimensions are explicitly constrained. If you intend for the element to maintain the 1440/230 ratio responsively, you may need to remove the fixedheightor useheight: auto.If this is intentional for documentation purposes or future flexibility, feel free to disregard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/layout/footer/_footer-1.scss` around lines 1776 - 1783, The .layer1 rule currently sets width: 100% and height: 250px alongside aspect-ratio: 1440 / 230, so remove the redundancy by either removing the fixed height (use height: auto or remove height) to allow aspect-ratio to control sizing responsively, or keep the fixed height and delete the aspect-ratio declaration; update the .layer1 selector accordingly (adjust the aspect-ratio, height, or both) depending on whether you want a responsive ratio or a fixed pixel height..husky/commit-msg (1)
4-4: Quote the shell variable to prevent word splitting.The
${1}parameter should be quoted to handle paths with spaces correctly, following shell scripting best practices.🛠️ Proposed fix
-npx --no -- commitlint --edit ${1} +npx --no -- commitlint --edit "${1}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.husky/commit-msg at line 4, The commit-msg hook invokes commitlint using the shell variable ${1} without quotes, which can break on paths with spaces; update the invocation in the commit hook (the line calling npx --no -- commitlint --edit ${1}) to quote the parameter so it becomes "${1}", ensuring the passed file path is treated as a single argument.eslint.config.mjs (1)
219-224: Enable type-aware linting for worker TypeScript files.Line 223 sets
project: null, which disables type-aware@typescript-eslintanalysis forworker/**/*.ts. Sincetsconfig.worker.jsonexists and is properly configured for worker files, change this to point to it for improved linting coverage.🔧 Suggested change
{ files: ["worker/**/*.ts"], languageOptions: { parserOptions: { - project: null, + project: "./tsconfig.worker.json", }, globals: { ...globals.serviceworker, ServiceWorkerGlobalScope: "readonly", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.mjs` around lines 219 - 224, The ESLint config for the worker files currently sets languageOptions.parserOptions.project to null (in the config block with files: ["worker/**/*.ts"]), which disables type-aware `@typescript-eslint` checks; update that project field to point to the worker tsconfig (e.g., "tsconfig.worker.json") so the rule set for worker/**/*.ts uses type-aware linting via the proper TypeScript project file (ensure the string matches the tsconfig filename used by your build).CONTRIBUTING.md (1)
19-19: Minor wording suggestion.Consider simplifying "any and all" to just "all" for conciseness.
✏️ Suggested wording
-- Fix any and all SonarQube issues before opening a PR. +- Fix all SonarQube issues before opening a PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` at line 19, Replace the phrase "Fix any and all SonarQube issues before opening a PR." with a more concise wording such as "Fix all SonarQube issues before opening a PR." — locate the exact line containing that sentence in CONTRIBUTING.md and update the text accordingly.components/elements/Countdown.tsx (1)
40-40: Prefer removing “what” comments here.Lines 40 and 54 are inline comments describing behavior that is already clear from the code path.
As per coding guidelines, "Code must be self-documenting. Only explain why non-obvious decisions were made in comments. DO NOT add inline comments explaining what code does."
Also applies to: 54-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/elements/Countdown.tsx` at line 40, Remove the redundant "what" inline comments in the Countdown component that state obvious behavior (e.g., the comments near the initial setTimeRemaining call in useEffect and the similar comment later); edit the Countdown function to delete those two comments so the code remains self-documenting, leaving only any necessary "why" comments if there are non-obvious decisions around useEffect, setTimeRemaining, or cleanup logic.__tests__/config/navigation.test.ts (2)
130-137: Test coverage reduced forgetNavLinksWithYear.The test previously verified all year-specific links but now only checks the first two (
speakers,talks). The third item (Schedule) is not validated.♻️ Add assertion for Schedule
years.forEach((year) => { const result = getNavLinksWithYear(year); expect(result[0].href).toBe(`/${year}/speakers`); expect(result[1].href).toBe(`/${year}/talks`); + expect(result[2].href).toBe(`/${year}/schedule`); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/config/navigation.test.ts` around lines 130 - 137, The test for getNavLinksWithYear no longer asserts the third "Schedule" link; update the test inside the years.forEach to also assert result[2].href equals `/${year}/schedule` (and/or check result[2].label === "Schedule" if labels exist) so all year-specific links (speakers, talks, schedule) are covered; modify the it block that calls getNavLinksWithYear to include this additional expectation.
146-154: Incomplete assertion forgetAllMainNavLinks.The test checks 6 items but
getAllMainNavLinksshould return 7 items (4 main + 3 year-specific). TheScheduleitem is missing from validation.♻️ Add missing assertion
expect(result[3].label).toBe("Travel"); expect(result[4].label).toBe("Speakers"); expect(result[5].label).toBe("Talks"); + expect(result[6].label).toBe("Schedule");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/config/navigation.test.ts` around lines 146 - 154, The test for getAllMainNavLinks is missing validation for the 'Schedule' item and doesn't assert the expected total count; update the test that calls getAllMainNavLinks() to assert result.length === 7 and add an assertion that one of the returned items has label "Schedule" (or assert the exact index if the order is known) so the 7th navigation entry is validated alongside the existing checks for About Us, Code of Conduct, Sponsors, Travel, Speakers, and Talks.config/editions/2024.ts (1)
15-19: Fragile dependency on array ordering.Using
newsDropdownLinks.slice(1)assumes the first item is always CFP. IfnewsDropdownLinksorder changes inconfig/navigation/index.ts, this will silently include the wrong items or duplicate entries.Consider filtering by label or using a more explicit approach:
♻️ Suggested approach
news: [ { label: "CFP", href: "/cfp", requiresYear: true, condition: "hasCfp" }, { label: "KCD", href: "/kcd", requiresYear: false }, - ...newsDropdownLinks.slice(1), + ...newsDropdownLinks.filter((link) => link.label !== "CFP"), ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/editions/2024.ts` around lines 15 - 19, The news array currently appends newsDropdownLinks.slice(1), which relies on ordering and can include wrong or duplicate items if newsDropdownLinks changes; update the construction of news to explicitly include or exclude items by property (e.g., filter newsDropdownLinks by label or an id) instead of slicing — inside the module that defines news replace the slice usage with a filter that removes the "CFP" entry (or selects only the intended labels), referencing the existing symbols news and newsDropdownLinks to locate and change the logic so the CFP entry is not duplicated and ordering is not assumed.lib/shared/navigation.ts (1)
40-41: Prefer direct property access overObject.entries().find()for known keys.Since
conditionsis typed asRecord<NavCondition, boolean>andlink.conditionisNavCondition | undefined, direct property access is simpler and more efficient:♻️ Simplify condition lookup
.filter((link) => { if (!link.condition) return true; - const conditionValue = Object.entries(conditions).find(([key]) => key === link.condition)?.[1]; - return !!conditionValue; + return conditions[link.condition]; })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/shared/navigation.ts` around lines 40 - 41, Replace the Object.entries/find lookup with direct property access: instead of using Object.entries(conditions).find(...) derive the boolean by checking link.condition and indexing into conditions (e.g., return !!(link.condition && conditions[link.condition])); this keeps the same behavior when link.condition is undefined but is simpler and more efficient.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/property/useSchedule.test.ts`:
- Around line 4-11: The test in describe("Property Tests for Schedule Data")
simply asserts Array.isArray on data generated by fc.array (using fc.array and
fc.record) which is tautological; replace it with a meaningful property test
that imports and exercises your schedule logic (for example call
processScheduleData or the real function that transforms/validates schedule
items) and assert invariants (e.g., length preserved, times sorted, titles
normalized), or remove the placeholder test entirely if no property tests exist
yet; locate the test by the describe/it block and references to fc.assert,
fc.property, fc.array and fc.record and either import the actual function (e.g.,
processScheduleData) and assert its properties, or delete the it(...) block.
In @.github/workflows/ci.yml:
- Around line 25-26: The "Security Audit" CI step currently uses the command
"npm audit --audit-level=high || true" which forces a successful exit; remove
the "|| true" fallback so the step fails on high-severity findings (i.e., run
"npm audit --audit-level=high" or otherwise ensure the step does not ignore
non-zero exit codes) to make the Security Audit gate block merges when high
vulnerabilities are found.
In `@ARCHITECTURE.md`:
- Around line 14-18: In ARCHITECTURE.md update the fenced code block that
contains the ASCII diagram starting with "Sessionize API ──► hooks/ ──►
app/[year]/ pages" to include a language identifier (e.g. change the opening ```
to ```text) so the block is lint-friendly; locate the block around that diagram
and replace the fence accordingly.
In `@commitlint.config.js`:
- Line 1: The config currently uses ESM syntax ("export default { extends: [...]
}") but the repo isn't in ESM mode; update commitlint config to use CommonJS by
replacing the ESM export with module.exports = { extends:
["@commitlint/config-conventional"] }; or alternatively enable ESM by adding
"type": "module" to package.json so Node will accept the existing export
default; target the symbol "export default" in commitlint.config.js or the
"type" field in package.json to apply the chosen fix.
In `@components/elements/Countdown.tsx`:
- Around line 32-38: Guard against invalid eventDate by validating the timestamp
and avoiding NaN propagation: in the calculateTimeDif function (and when
computing targetTime) parse eventDate into a date value and check
Number.isFinite(targetTime) or isNaN(targetTime); if invalid, return 0 (or a
sensible fallback) so timeDif never becomes NaN. Move the targetTime calculation
inside calculateTimeDif or revalidate it there to handle prop changes, and
ensure any rendering logic uses the safe non-negative value from
calculateTimeDif.
In `@components/layout/footer/Footer8.tsx`:
- Around line 88-91: Fix the spelling error in the footer link text inside the
Footer8 component: change the hardcoded string "Barceclona" to "Barcelona" where
the JSX renders <Link> with the <Image> icon so the location reads "World Trade
Center, Barcelona".
In `@components/layout/Menu.tsx`:
- Around line 9-16: Wrap each Link inside an li so the ul's direct children are
li elements (update the JSX around the <ul className="sub-menu"> block to render
<li><Link ...>...</Link></li> for each entry), and fix the second link's href so
it matches the active-class check (change the href for the "Home Interior" Link
from "/#" to "/index-2" or alternatively adjust the pathname comparison in the
className logic) — update references to Link, pathname, and className in the
Menu component accordingly.
In `@cypress/e2e/home/home-editions.cy.ts`:
- Line 14: Remove the inline comment "// Venue and date are now in span elements
or nested divs, so we use cy.contains directly" from the test body in
home-editions.cy.ts; simply delete that comment line so the test reads without
explanatory "what" comments, leaving only the assertions (e.g., the
cy.contains(...) calls) which are self-documenting.
In `@lib/shared/navigation.ts`:
- Around line 43-51: The map callback that updates year-scoped URLs (the
anonymous function using link.requiresYear and year in lib/shared/navigation.ts)
should explicitly skip hash links: add a guard at the top of the map callback
that returns the original link immediately if link.href startsWith("#"), then
keep the existing logic (if link.requiresYear &&
!link.href.startsWith(`/${year}`) { ... }). This prevents producing malformed
URLs like `/${year}/#id` while preserving the year-prefixing behavior for other
links.
In `@package.json`:
- Line 28: The "check:docs" script currently copies AGENTS.md to CLAUDE.md
(mutating state) instead of validating equality; update the package.json
"check:docs" script so it only validates that both files exist and are identical
(e.g., use a read-only compare like diff/cmp/git diff and exit non-zero on
mismatch) rather than copying, and keep the existing existence checks for
AGENTS.md and CLAUDE.md; target the "check:docs" script entry in package.json to
implement this change.
In `@scorecard-report.md`:
- Around line 20-53: The published scorecard-report.md is stale: it claims
ARCHITECTURE.md, CLAUDE.md/AGENTS.md, and docs/adr/ are missing even though
those files are added in this PR; regenerate the report from the repository's
final state and commit the updated report. Re-run the project’s scorecard/report
generation step (the same command or CI job that produces scorecard-report.md),
validate that entries referencing ARCHITECTURE.md, CLAUDE.md/AGENTS.md, and
docs/adr/ now pass, and replace the stale scorecard-report.md with the freshly
generated output before merging.
In `@styles/layout/pages/event/_home-8.scss`:
- Around line 205-212: The .btn-area1 is currently hidden by bottom: -10px and
transform: translateY(100%); add a hover reveal or change its default position:
either add a rule like .event2-boxarea:hover .btn-area1 that resets transform to
translateY(0) (and optionally adjusts bottom) to reveal the button on hover, or
remove/modify bottom/transform on .btn-area1 to make it visible by default;
update the related CSS selectors (.btn-area1 and .event2-boxarea) accordingly
and ensure transitions (e.g., transition: transform .2s) are present for a
smooth reveal.
In `@worker/index.ts`:
- Around line 13-15: The activation promise passed to event.waitUntil is being
swallowed by a .catch that only logs via console.error, which makes activation
appear successful even on failure; update the handler so it does not resolve on
error—either remove the .catch or rethrow the error after logging (i.e., in the
.catch call log with console.error("Service worker activation failed:", error)
and then throw error) so the promise stays rejected and event.waitUntil sees the
failure; target the activation promise usage around event.waitUntil and the
existing .catch attached to it.
---
Outside diff comments:
In `@components/layout/header/Header8.tsx`:
- Around line 9-16: HeaderProps declares isSearch and handleSearch but Header8
does not use them; remove those two props from the HeaderProps interface and
from the Header8 parameter type so HeaderProps only contains scroll and
navigation (and update the function signature export default function Header8({
scroll, navigation }: Readonly<HeaderProps>) accordingly). Then update the
caller in DynamicHeaderWrapper.tsx to stop passing isSearch and handleSearch to
<Header8 ... /> (leave scroll and navigation as before) and remove any related
imports/usages there; also search for other callers of Header8 and update them
similarly.
In `@components/layout/Popup.tsx`:
- Around line 33-40: The cleanup currently passes new anonymous functions to
removeEventListener so the original click handlers (the inline arrow functions
added to closeBtn and noThanksBtn) are never removed; fix this by extracting
those inline handlers into stable references (e.g., const handleCloseClick =
(...) => { ... } and const handleNoThanksClick = (...) => { ... }), use those
variables when calling addEventListener on closeBtn and noThanksBtn, and then
call removeEventListener with the same handleCloseClick and handleNoThanksClick
in the return cleanup to properly detach the listeners.
In `@styles/layout/pages/_others.scss`:
- Around line 1697-1721: The .sponsor-card block in this file duplicates the
definition already provided by the imported stylesheet (`@import` "./brands");
remove this redundant .sponsor-card rule from styles/layout/pages/_others.scss
so the project uses the single canonical definition in _brands.scss, ensuring
you only delete the entire .sponsor-card rule (including its nested img and
&:hover rules) and leave the `@import` "./brands"; line intact.
---
Nitpick comments:
In `@__tests__/config/navigation.test.ts`:
- Around line 130-137: The test for getNavLinksWithYear no longer asserts the
third "Schedule" link; update the test inside the years.forEach to also assert
result[2].href equals `/${year}/schedule` (and/or check result[2].label ===
"Schedule" if labels exist) so all year-specific links (speakers, talks,
schedule) are covered; modify the it block that calls getNavLinksWithYear to
include this additional expectation.
- Around line 146-154: The test for getAllMainNavLinks is missing validation for
the 'Schedule' item and doesn't assert the expected total count; update the test
that calls getAllMainNavLinks() to assert result.length === 7 and add an
assertion that one of the returned items has label "Schedule" (or assert the
exact index if the order is known) so the 7th navigation entry is validated
alongside the existing checks for About Us, Code of Conduct, Sponsors, Travel,
Speakers, and Talks.
In @.husky/commit-msg:
- Line 4: The commit-msg hook invokes commitlint using the shell variable ${1}
without quotes, which can break on paths with spaces; update the invocation in
the commit hook (the line calling npx --no -- commitlint --edit ${1}) to quote
the parameter so it becomes "${1}", ensuring the passed file path is treated as
a single argument.
In `@components/elements/Countdown.tsx`:
- Line 40: Remove the redundant "what" inline comments in the Countdown
component that state obvious behavior (e.g., the comments near the initial
setTimeRemaining call in useEffect and the similar comment later); edit the
Countdown function to delete those two comments so the code remains
self-documenting, leaving only any necessary "why" comments if there are
non-obvious decisions around useEffect, setTimeRemaining, or cleanup logic.
In `@components/layout/Popup.tsx`:
- Around line 6-41: Replace the manual DOM manipulation in Popup (the useEffect
that uses getElementById/querySelector, closeBtn/noThanksBtn and style.display
toggles) with React state and handlers: add a local state like isVisible and
setIsVisible, use useEffect only to start/clear a timer that sets isVisible true
(clearTimeout on cleanup), remove addEventListener/removeEventListener logic,
and wire the close action to onClick handlers on the close element and the
"no-thanks" element in the Popup component JSX so visibility is controlled
declaratively by isVisible.
In `@config/editions/2024.ts`:
- Around line 15-19: The news array currently appends
newsDropdownLinks.slice(1), which relies on ordering and can include wrong or
duplicate items if newsDropdownLinks changes; update the construction of news to
explicitly include or exclude items by property (e.g., filter newsDropdownLinks
by label or an id) instead of slicing — inside the module that defines news
replace the slice usage with a filter that removes the "CFP" entry (or selects
only the intended labels), referencing the existing symbols news and
newsDropdownLinks to locate and change the logic so the CFP entry is not
duplicated and ordering is not assumed.
In `@CONTRIBUTING.md`:
- Line 19: Replace the phrase "Fix any and all SonarQube issues before opening a
PR." with a more concise wording such as "Fix all SonarQube issues before
opening a PR." — locate the exact line containing that sentence in
CONTRIBUTING.md and update the text accordingly.
In `@eslint.config.mjs`:
- Around line 219-224: The ESLint config for the worker files currently sets
languageOptions.parserOptions.project to null (in the config block with files:
["worker/**/*.ts"]), which disables type-aware `@typescript-eslint` checks; update
that project field to point to the worker tsconfig (e.g.,
"tsconfig.worker.json") so the rule set for worker/**/*.ts uses type-aware
linting via the proper TypeScript project file (ensure the string matches the
tsconfig filename used by your build).
In `@lib/shared/navigation.ts`:
- Around line 40-41: Replace the Object.entries/find lookup with direct property
access: instead of using Object.entries(conditions).find(...) derive the boolean
by checking link.condition and indexing into conditions (e.g., return
!!(link.condition && conditions[link.condition])); this keeps the same behavior
when link.condition is undefined but is simpler and more efficient.
In `@styles/layout/footer/_footer-1.scss`:
- Around line 1776-1783: The .layer1 rule currently sets width: 100% and height:
250px alongside aspect-ratio: 1440 / 230, so remove the redundancy by either
removing the fixed height (use height: auto or remove height) to allow
aspect-ratio to control sizing responsively, or keep the fixed height and delete
the aspect-ratio declaration; update the .layer1 selector accordingly (adjust
the aspect-ratio, height, or both) depending on whether you want a responsive
ratio or a fixed pixel height.
In `@styles/layout/pages/_brands.scss`:
- Around line 394-418: Duplicate .sponsor-card rules are defined in both
_brands.scss and _others.scss; remove the redundant block from one file to avoid
duplication and maintenance drift. Keep the authoritative definition (preferably
the one in _brands.scss) and delete the matching .sponsor-card selector block in
_others.scss (including its &:hover and img nested rules), or alternatively
remove the one in _brands.scss and ensure _others.scss still imports "./brands";
update any imports if you move the definition so only a single .sponsor-card
selector remains in the codebase.
In `@styles/layout/pages/_others.scss`:
- Around line 1683-1694: Remove the duplicated CSS rules for
.sponsors-section-heading and .sponsor-category-title from this file
(_others.scss) and keep the single source of truth in the existing _brands.scss;
locate the two class blocks shown and delete them, then run a quick grep/IDE
search for .sponsors-section-heading and .sponsor-category-title to confirm no
other duplicates or missing references remain and rebuild to verify styles are
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a9c4098-54ff-49df-a12a-7c59ab61bc36
⛔ Files ignored due to path filters (7)
__tests__/components/elements/__snapshots__/AboutCounter.test.tsx.snapis excluded by!**/*.snap__tests__/components/elements/__snapshots__/BuyTicketButton.test.tsx.snapis excluded by!**/*.snap__tests__/components/elements/__snapshots__/CircleText.test.tsx.snapis excluded by!**/*.snap__tests__/components/ui/__snapshots__/Modal.test.tsx.snapis excluded by!**/*.snap__tests__/snapshots/__snapshots__/Menu.test.tsx.snapis excluded by!**/*.snappackage-lock.jsonis excluded by!**/package-lock.jsonpublic/assets/img/all-images/team/nacho-cougil.jpgis excluded by!**/*.jpg
📒 Files selected for processing (65)
.github/CODEOWNERS.github/PULL_REQUEST_TEMPLATE.md.github/dependabot.yml.github/workflows/ci.yml.github/workflows/e2e.yml.github/workflows/scorecard.yml.gitignore.husky/commit-msg.husky/pre-pushAGENTS.mdARCHITECTURE.mdCONTRIBUTING.mdREADME.mdSECURITY.md__tests__/components/Footer8.test.tsx__tests__/components/elements/AboutCounter.test.tsx__tests__/components/elements/BuyTicketButton.test.tsx__tests__/components/elements/CircleText.test.tsx__tests__/components/ui/Modal.test.tsx__tests__/config/navigation.test.ts__tests__/fuzz/slugify.fuzz.ts__tests__/property/useSchedule.test.ts__tests__/snapshots/Menu.test.tsxany-errors-with-files.txtapp/[year]/cfp/cfpData2026.tscommitlint.config.jscomponents/BackgroundCarousel.tsxcomponents/elements/Countdown.tsxcomponents/layout/ClientLayout.tsxcomponents/layout/Layout.tsxcomponents/layout/Menu.tsxcomponents/layout/PageHeader.tsxcomponents/layout/PageSidebar.tsxcomponents/layout/Popup.tsxcomponents/layout/footer/Footer8.tsxcomponents/layout/header/Header8.tsxconfig/editions/2023.tsconfig/editions/2024.tsconfig/editions/2025.tsconfig/editions/2026.tsconfig/editions/types.tsconfig/navigation/index.tsconfig/navigation/types.tscypress/e2e/home/home-editions.cy.tsdocs/adr/0001-caching-strategy-for-gcs-images.mddocs/rfcs/0001-design-before-code.mdeslint.config.mjslib/shared/navigation.tsnext.config.mjspackage.jsonpublic/assets/css/main.cssscorecard-badge.jsonscorecard-report.mdstryker.config.jsonstyles/layout/footer/_footer-1.scssstyles/layout/pages/_brands.scssstyles/layout/pages/_others.scssstyles/layout/pages/event/_home-8.scssteam/TeamMembers.tstsconfig.jsontsconfig.worker.jsontype-check-output.txttype-errors.txtvercel.jsonworker/index.ts
💤 Files with no reviewable changes (3)
- type-check-output.txt
- type-errors.txt
- any-errors-with-files.txt
| - name: Security Audit | ||
| run: npm audit --audit-level=high || true |
There was a problem hiding this comment.
Make the security audit step actually gate high vulnerabilities.
Line 26 always exits successfully because of || true, so high-severity findings won’t block merges.
🔧 Suggested fix
- - name: Security Audit
- run: npm audit --audit-level=high || true
+ - name: Security Audit
+ run: npm audit --audit-level=high📝 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.
| - name: Security Audit | |
| run: npm audit --audit-level=high || true | |
| - name: Security Audit | |
| run: npm audit --audit-level=high |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 25 - 26, The "Security Audit" CI step
currently uses the command "npm audit --audit-level=high || true" which forces a
successful exit; remove the "|| true" fallback so the step fails on
high-severity findings (i.e., run "npm audit --audit-level=high" or otherwise
ensure the step does not ignore non-zero exit codes) to make the Security Audit
gate block merges when high vulnerabilities are found.
| ``` | ||
| Sessionize API ──► hooks/ ──► app/[year]/ pages | ||
| ▲ | ||
| config/editions/ ─────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced code block.
Line 14 opens a fenced block without a language, which trips markdown linting (MD040).
Suggested fix
-```
+```text
Sessionize API ──► hooks/ ──► app/[year]/ pages
▲
config/editions/ ─────────────────┘</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>
[warning] 14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @ARCHITECTURE.md around lines 14 - 18, In ARCHITECTURE.md update the fenced
code block that contains the ASCII diagram starting with "Sessionize API ──►
hooks/ ──► app/[year]/ pages" to include a language identifier (e.g. change the
opening totext) so the block is lint-friendly; locate the block around
that diagram and replace the fence accordingly.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| @@ -0,0 +1 @@ | |||
| export default { extends: ["@commitlint/config-conventional"] }; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify module mode and commitlint hook wiring (read-only).
fd -HI '^package.json$' --exec cat {}
echo "---- package.json module type ----"
rg -n '"type"\s*:\s*"module"' package.json
echo "---- commitlint usage ----"
rg -n 'commitlint|@commitlint/config-conventional|@commitlint/cli' -C2 package.json .husky/commit-msg
echo "---- commitlint config files ----"
fd -HI 'commitlint.config.*'Repository: anyulled/devbcn-nextjs
Length of output: 4692
Fix commitlint.config.js to use CommonJS syntax or enable ESM mode.
The config uses export default (ESM) but package.json lacks "type": "module", so Node.js treats .js files as CommonJS. commitlint will fail to load this config and break the commit-msg hook. Either change to module.exports = { extends: ["@commitlint/config-conventional"] }; or add "type": "module" to package.json.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@commitlint.config.js` at line 1, The config currently uses ESM syntax
("export default { extends: [...] }") but the repo isn't in ESM mode; update
commitlint config to use CommonJS by replacing the ESM export with
module.exports = { extends: ["@commitlint/config-conventional"] }; or
alternatively enable ESM by adding "type": "module" to package.json so Node will
accept the existing export default; target the symbol "export default" in
commitlint.config.js or the "type" field in package.json to apply the chosen
fix.
| const targetTime = new Date(eventDate).getTime(); | ||
|
|
||
| const calculateTimeDif = () => { | ||
| const now = Date.now(); | ||
| const diff = targetTime - now; | ||
| return Math.max(0, diff); | ||
| }; |
There was a problem hiding this comment.
Guard against invalid eventDate input.
Line 32 can produce NaN for invalid date strings, which then propagates to timeDif and renders NaN parts.
Suggested fix
- const targetTime = new Date(eventDate).getTime();
+ const targetTime = new Date(eventDate).getTime();
+ if (Number.isNaN(targetTime)) {
+ setTimeDif(0);
+ return;
+ }📝 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.
| const targetTime = new Date(eventDate).getTime(); | |
| const calculateTimeDif = () => { | |
| const now = Date.now(); | |
| const diff = targetTime - now; | |
| return Math.max(0, diff); | |
| }; | |
| const targetTime = new Date(eventDate).getTime(); | |
| if (Number.isNaN(targetTime)) { | |
| setTimeDif(0); | |
| return; | |
| } | |
| const calculateTimeDif = () => { | |
| const now = Date.now(); | |
| const diff = targetTime - now; | |
| return Math.max(0, diff); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/elements/Countdown.tsx` around lines 32 - 38, Guard against
invalid eventDate by validating the timestamp and avoiding NaN propagation: in
the calculateTimeDif function (and when computing targetTime) parse eventDate
into a date value and check Number.isFinite(targetTime) or isNaN(targetTime); if
invalid, return 0 (or a sensible fallback) so timeDif never becomes NaN. Move
the targetTime calculation inside calculateTimeDif or revalidate it there to
handle prop changes, and ensure any rendering logic uses the safe non-negative
value from calculateTimeDif.
| .map((link) => { | ||
| if (link.requiresYear && !link.href.startsWith(`/${year}`)) { | ||
| return { | ||
| ...link, | ||
| href: `/${year}${link.href.startsWith("/") ? "" : "/"}${link.href}`, | ||
| }; | ||
| } | ||
| return link; | ||
| }); |
There was a problem hiding this comment.
Hash links will be incorrectly prefixed with year.
Links with href starting with # (e.g., #sponsors) have requiresYear: false in mainNavLinks, but if any hash link ever had requiresYear: true, this logic would produce malformed URLs like /${year}/#sponsors.
Consider adding an explicit check to skip hash links:
🛡️ Defensive fix for hash links
.map((link) => {
- if (link.requiresYear && !link.href.startsWith(`/${year}`)) {
+ if (link.requiresYear && !link.href.startsWith(`/${year}`) && !link.href.startsWith("#")) {
return {
...link,
href: `/${year}${link.href.startsWith("/") ? "" : "/"}${link.href}`,
};
}
return link;
});📝 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.
| .map((link) => { | |
| if (link.requiresYear && !link.href.startsWith(`/${year}`)) { | |
| return { | |
| ...link, | |
| href: `/${year}${link.href.startsWith("/") ? "" : "/"}${link.href}`, | |
| }; | |
| } | |
| return link; | |
| }); | |
| .map((link) => { | |
| if (link.requiresYear && !link.href.startsWith(`/${year}`) && !link.href.startsWith("#")) { | |
| return { | |
| ...link, | |
| href: `/${year}${link.href.startsWith("/") ? "" : "/"}${link.href}`, | |
| }; | |
| } | |
| return link; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/shared/navigation.ts` around lines 43 - 51, The map callback that updates
year-scoped URLs (the anonymous function using link.requiresYear and year in
lib/shared/navigation.ts) should explicitly skip hash links: add a guard at the
top of the map callback that returns the original link immediately if link.href
startsWith("#"), then keep the existing logic (if link.requiresYear &&
!link.href.startsWith(`/${year}`) { ... }). This prevents producing malformed
URLs like `/${year}/#id` while preserving the year-prefixing behavior for other
links.
| "e2e:run": "start-server-and-test start http://localhost:3000 cypress:run" | ||
| "e2e:run": "start-server-and-test start http://localhost:3000 cypress:run", | ||
| "build:docs": "typedoc", | ||
| "check:docs": "bash -c 'echo \"Checking docs sync...\" && ([ -f AGENTS.md ] && [ -f CLAUDE.md ] || cp AGENTS.md CLAUDE.md)'" |
There was a problem hiding this comment.
check:docs is mutating state instead of validating it.
Line 28 copies AGENTS.md into CLAUDE.md during a check step. That masks drift and introduces side effects in CI/local checks.
Suggested fix (read-only validation)
- "check:docs": "bash -c 'echo \"Checking docs sync...\" && ([ -f AGENTS.md ] && [ -f CLAUDE.md ] || cp AGENTS.md CLAUDE.md)'"
+ "check:docs": "bash -euo pipefail -c 'echo \"Checking docs sync...\"; test -f AGENTS.md; test -f CLAUDE.md; cmp -s AGENTS.md CLAUDE.md'"📝 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.
| "check:docs": "bash -c 'echo \"Checking docs sync...\" && ([ -f AGENTS.md ] && [ -f CLAUDE.md ] || cp AGENTS.md CLAUDE.md)'" | |
| "check:docs": "bash -euo pipefail -c 'echo \"Checking docs sync...\"; test -f AGENTS.md; test -f CLAUDE.md; cmp -s AGENTS.md CLAUDE.md'" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 28, The "check:docs" script currently copies AGENTS.md
to CLAUDE.md (mutating state) instead of validating equality; update the
package.json "check:docs" script so it only validates that both files exist and
are identical (e.g., use a read-only compare like diff/cmp/git diff and exit
non-zero on mismatch) rather than copying, and keep the existing existence
checks for AGENTS.md and CLAUDE.md; target the "check:docs" script entry in
package.json to implement this change.
| ## Architectural Documentation (0%) | ||
|
|
||
| ### [FAIL] Architecture Documentation (0/5) | ||
|
|
||
| _matklad ARCHITECTURE.md guide_ | ||
|
|
||
| **Evidence**: No architecture documentation found | ||
|
|
||
| **Remediation**: Create ARCHITECTURE.md at repo root following matklad's pattern: short, stable, focused on module boundaries and constraints. | ||
|
|
||
| ### [FAIL] Agent Instructions (0/5) | ||
|
|
||
| _OpenAI Harness Engineering (2026)_ | ||
|
|
||
| **Evidence**: No AI agent instruction files found | ||
|
|
||
| **Remediation**: Create CLAUDE.md or AGENTS.md with project context, code style, and constraints so AI agents produce consistent output. | ||
|
|
||
| ### [FAIL] Architecture Decision Records (0/3) | ||
|
|
||
| _DORA 2025 Report - AI-accessible documentation_ | ||
|
|
||
| **Evidence**: No Architecture Decision Records found | ||
|
|
||
| **Remediation**: Create docs/adr/ directory with numbered markdown decision records. Use adr-tools or a simple template. | ||
|
|
||
| ### [FAIL] Module Boundary Documentation (0/4) | ||
|
|
||
| _matklad ARCHITECTURE.md - constraints as absences_ | ||
|
|
||
| **Evidence**: No module boundary constraints documented | ||
|
|
||
| **Remediation**: Document which modules must NOT depend on each other in ARCHITECTURE.md. Example: 'The fields crate never depends on any other workspace crate.' | ||
|
|
There was a problem hiding this comment.
Scorecard findings are stale and currently incorrect.
Lines 26, 34, 42, and 50 assert missing documentation that exists in this PR (e.g., ARCHITECTURE.md). This makes the published score/report misleading.
Please regenerate the report from the final repository state before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scorecard-report.md` around lines 20 - 53, The published scorecard-report.md
is stale: it claims ARCHITECTURE.md, CLAUDE.md/AGENTS.md, and docs/adr/ are
missing even though those files are added in this PR; regenerate the report from
the repository's final state and commit the updated report. Re-run the project’s
scorecard/report generation step (the same command or CI job that produces
scorecard-report.md), validate that entries referencing ARCHITECTURE.md,
CLAUDE.md/AGENTS.md, and docs/adr/ now pass, and replace the stale
scorecard-report.md with the freshly generated output before merging.
|
Verify each finding against the current code and only fix it if needed. In |
|
Verify each finding against the current code and only fix it if needed. In |
|
Verify each finding against the current code and only fix it if needed. In |
Agreed. The property test was a tautological placeholder checking array generation instead of testing schedule logic. I've deleted it. |
Fixed. I've updated the |
Done. Added a hover reveal by setting |
- Optimized Countdown component to parse eventDate once instead of every interval tick. - Removed tautological property test for `useSchedule`. - Ensured service worker activation promise propagates errors by rethrowing. - Fixed UI hidden button issue in section 8 by adding hover reveal transition. Co-authored-by: anyulled <100741+anyulled@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@styles/layout/pages/event/_home-8.scss`:
- Around line 174-180: The CTA is only revealed on :hover so touch and keyboard
users never see .btn-area1; update the CSS around the .content-area/.btn-area1
rule to also reveal the button on keyboard/focus and touch interaction paths by
adding equivalent selectors such as .content-area:focus,
.content-area:focus-within and .content-area:active (or the parent card's
:focus/:focus-within) alongside :hover so transform: translateY(0) is applied
for keyboard and touch; ensure the default off-canvas transform on .btn-area1
remains but is overridden by these additional selectors so accessibility is
preserved without changing markup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6841f4c3-ea2b-40b9-9bc2-14340c81e6b5
📒 Files selected for processing (2)
styles/layout/pages/event/_home-8.scssworker/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- worker/index.ts
| &:hover { | ||
| .content-area { | ||
| .btn-area1 { | ||
| transform: translateY(0); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Button remains hidden on touch/keyboard-only interaction paths.
Line 174 reveals the CTA only on :hover, while Line 220 keeps it off-canvas by default. On touch devices (and some keyboard flows), hover may not occur, so the Buy Ticket CTA can stay inaccessible.
♿ Suggested CSS adjustment
.event2-boxarea {
- &:hover {
+ &:hover,
+ &:focus-within {
.content-area {
.btn-area1 {
transform: translateY(0);
}
}
}
.content-area {
.btn-area1 {
position: absolute;
bottom: -10px;
left: 0;
right: 0;
display: flex;
justify-content: center;
transform: translateY(100%);
transition: transform 0.3s ease-in-out;
+
+ `@media` (hover: none), (pointer: coarse) {
+ transform: translateY(0);
+ }
}
}
}Also applies to: 220-231
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@styles/layout/pages/event/_home-8.scss` around lines 174 - 180, The CTA is
only revealed on :hover so touch and keyboard users never see .btn-area1; update
the CSS around the .content-area/.btn-area1 rule to also reveal the button on
keyboard/focus and touch interaction paths by adding equivalent selectors such
as .content-area:focus, .content-area:focus-within and .content-area:active (or
the parent card's :focus/:focus-within) alongside :hover so transform:
translateY(0) is applied for keyboard and touch; ensure the default off-canvas
transform on .btn-area1 remains but is overridden by these additional selectors
so accessibility is preserved without changing markup.
|
Closing as duplicate of #64. |
1 participant
💡 What:
components/elements/Countdown.tsxto initializetimeDifstate asnull.useEffecton component mount.nulluntil the client-side calculation is complete.🎯 Why:
timeDifusingDate.now(). SinceDate.now()differs between the server (SSR) and the client (hydration), this caused a hydration mismatch error, forcing React to discard the server-rendered HTML and re-render the entire tree, which is a performance penalty (TTI).📊 Impact:
🔬 Measurement:
Date.now()is no longer used inuseStateinitializer.PR created automatically by Jules for task 16632541641760643710 started by @anyulled
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Performance