Skip to content

Added Thanksgiving Theme - #1917

Open
FROSTYKOR wants to merge 1 commit into
tjcsl:devfrom
FROSTYKOR:feature/thanksgiving-theme
Open

FROSTYKOR wants to merge 1 commit into
tjcsl:devfrom
FROSTYKOR:feature/thanksgiving-theme

Conversation

@FROSTYKOR

Copy link
Copy Markdown

Added a Thanksgiving theme.
It starts a week before Thanksgiving until the day is over.
Custom autumn colors, and falling leaves.
Leaves do not stack as they are emojis unlike the snowflakes, making it super laggy.
Emojis may render differently per OS, but trivial issue compared to the benefit of not having to load assets.
There is also a turkey trotting at the bottom!
Toggle on/off button for the theme. Preference is stored.

-Ben Ahn 5/22/2026

Closes issue #1852

@FROSTYKOR
FROSTYKOR requested a review from a team as a code owner May 22, 2026 04:08
@FROSTYKOR

Copy link
Copy Markdown
Author

Hold up, fixing CI / Build

@FROSTYKOR
FROSTYKOR marked this pull request as draft May 22, 2026 04:11
@FROSTYKOR
FROSTYKOR force-pushed the feature/thanksgiving-theme branch 2 times, most recently from 008311f to affbe6c Compare May 22, 2026 04:25
@FROSTYKOR
FROSTYKOR marked this pull request as ready for review May 22, 2026 04:31
@coveralls

coveralls commented May 22, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 78.851% (-0.009%) from 78.86% — FROSTYKOR:feature/thanksgiving-theme into tjcsl:dev

@FROSTYKOR

Copy link
Copy Markdown
Author

Done. Fixed CI / build issue. Codacy has some stuff but I got it done.

@FROSTYKOR
FROSTYKOR marked this pull request as draft May 22, 2026 04:36
@FROSTYKOR
FROSTYKOR force-pushed the feature/thanksgiving-theme branch from abbf8e7 to cd587a3 Compare May 22, 2026 04:38
@FROSTYKOR

Copy link
Copy Markdown
Author

I apologize for the many pushes, there were some issues I couldn't tell where it was wrong

@FROSTYKOR
FROSTYKOR force-pushed the feature/thanksgiving-theme branch 3 times, most recently from 6e9ab1d to 2612e87 Compare May 22, 2026 04:48
@FROSTYKOR
FROSTYKOR marked this pull request as ready for review May 22, 2026 04:54
@FROSTYKOR

FROSTYKOR commented May 22, 2026

Copy link
Copy Markdown
Author

Done! Finally, sorry for the delay and commits. Feature for issue #1852 is finally added.

@aarushtools aarushtools left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase latest thanks

@FROSTYKOR
FROSTYKOR force-pushed the feature/thanksgiving-theme branch from 2612e87 to db9250e Compare June 17, 2026 01:53
@FROSTYKOR
FROSTYKOR requested a review from aarushtools June 21, 2026 00:34
@FROSTYKOR
FROSTYKOR force-pushed the feature/thanksgiving-theme branch 3 times, most recently from 2e55a52 to d62eea9 Compare August 23, 2026 19:58
@FROSTYKOR
FROSTYKOR force-pushed the feature/thanksgiving-theme branch from d62eea9 to 8af19c4 Compare August 23, 2026 20:02
@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

Not ready to merge until the mobile navigation lookup is made safe and the opt-out lifetime covers the entire scheduled theme period.

The mobile failure and the premature preference expiry were each reproduced in Chromium using the repository JavaScript, with focused before-and-after comparisons isolating their causes.

Files Needing Attention: Update intranet/static/themes/thanksgiving/thanksgiving.js and the matching logic in intranet/static/themes/thanksgiving/thanksgiving-cookie.js; preserve alignment with the activation interval in intranet/utils/helpers.py.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P1 finding and linked it to the corresponding review comment.
  • T-Rex produced a proof for a posted P2 finding and linked it to the corresponding review comment.
  • T-Rex published a general-contract-validation-proof detailing the mobile-branch regression and the unguarded null-dereference when there is no navigation list.
  • T-Rex published a general-contract-validation-proof describing the Thanksgiving cookie expiry logic and the eight-day window behavior.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. General comment

    P1 Mobile Thanksgiving theme crashes when the page shell has no ul.nav

    • Bug
      • On viewports below 1000px, a page shell that lacks ul.nav causes the theme's ready handler to throw TypeError: Cannot read properties of null (reading 'appendChild'). The desktop/header toggle is hidden immediately before this call, and no mobile toggle is rendered.
    • Cause
      • document.querySelector('ul.nav') can return null, but line 66 unconditionally calls appendChild on its result.
    • Fix
      • Store the query result and append only when it exists, e.g. var nav = document.querySelector('ul.nav'); if (nav) nav.appendChild(liOff);. If a mobile toggle is required on all page shells, provide a deliberate fallback container rather than assuming ul.nav.

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P2 Thanksgiving disable cookie expires before the inclusive activation window ends

    • Bug
      • At the start of the server's Nov 19–26, 2026 inclusive Thanksgiving window, the real browser execution writes disable-thanksgiving=1 with expiry at 2026-11-26T00:00:00.000Z. Because the server remains active throughout Nov 26 and only suppresses theme files while this cookie is 1, a subsequent request on Thanksgiving Day re-enables the theme before the scheduled window ends.
    • Cause
      • intranet/static/themes/thanksgiving/thanksgiving.js:71-72 uses a seven-day cookie lifetime, whereas the server activation range in intranet/utils/helpers.py:267-271 spans eight inclusive calendar dates.
    • Fix
      • Set the disable cookie for at least eight days (and make the matching thanksgiving-cookie.js:18-19 change) so a disable at the opening instant remains in effect through the final inclusive day.

    T-Rex Ran code and verified through T-Rex

Fix all with Greploop Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix: recovered thanksgiving ver" | Re-trigger Greptile

'<i class="fas fa-leaf" style="font-size:16pt;position:relative;top:3px;left:6px;"></i>' +
'<span style="position:relative;bottom:9px;left:15px;">Turn Off<br>Thanksgiving Theme</span>' +
'</a>';
document.querySelector('ul.nav').appendChild(liOff);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Mobile navigation null dereference

At widths below 1000px, this assumes every page shell has ul.nav. On a shell without that element, querySelector returns null and appendChild throws. The header toggle has already been hidden, so no mobile control is available; thanksgiving-cookie.js:13 contains the same unguarded append and also prevents users from re-enabling the theme. Guard the lookup in both scripts, with an intentional fallback container if these controls must appear on nav-less pages.

Artifacts

▶ Current Thanksgiving theme at 800px on a shell without ul.nav

  • Chromium recording of the current source on a page whose header has no `ul.nav`; the ready handler reaches the missing-nav branch and throws, so no mobile navigation toggle appears.

Current Thanksgiving theme before-case poster frame

  • Poster frame for the current-source browser reproduction on the shell without `ul.nav`.

▶ Null-guarded comparison at 800px on a shell without ul.nav

  • Chromium recording of the same shell with a test-only null guard replacing line 66; execution completes without a page error, confirming the unguarded append is the failure point.

Null-guarded comparison poster frame

  • Poster frame for the null-guarded comparison browser reproduction.

Focused Playwright reproduction script

  • The exact Node Playwright script executed against the repository Thanksgiving source and the same deliberately nav-less shell for both recordings.

Observed Playwright reproduction output

  • Captured command output shows the current-source TypeError and comparison run without errors, together with DOM observations from both executions.

View artifacts

T-Rex Ran code and verified through T-Rex

Fix in Codex Fix in Claude Code

Comment on lines +71 to +72
var enabled = Cookies.get('disable-thanksgiving') == '1' ? '0' : '1';
Cookies.set('disable-thanksgiving', enabled, { expires: 7 });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Opt-out cookie expires before Thanksgiving ends

The server activates the theme for eight inclusive calendar dates, from seven days before Thanksgiving through the end of Thanksgiving Day, but this preference lasts only seven days. A user who turns the theme off when the period opens receives a cookie that expires at the start of Thanksgiving Day, so later requests that day restore the theme without another choice by the user. Extend the lifetime in both toggle scripts so it covers the complete inclusive activation period.

Artifacts

Chromium test script for Thanksgiving disable-cookie expiry

  • The exact focused Playwright test reads and executes the repository's Thanksgiving toggle and vendored cookie implementation, demonstrating the expiration comparison.

Successful Chromium expiry test output

  • Command metadata and observed current-versus-eight-day cookie expiration output show that the seven-day cookie expires at Thanksgiving's opening instant.

▶ Current seven-day Thanksgiving disable cookie expires at Thanksgiving start

  • Headless Chromium executes the current repository code at the active-window opening instant and displays expiry at Nov 26 00:00 UTC, proving the premature re-enable condition.

Poster frame showing the current cookie expiry at Thanksgiving start

  • The current-code poster shows the captured cookie write and that it expires before the inclusive Thanksgiving Day end.

▶ Eight-day Thanksgiving disable cookie remains through the server window

  • Headless Chromium executes the controlled eight-day comparison and shows expiry after Thanksgiving Day, demonstrating the minimal correction.

Poster frame showing the eight-day cookie expiry after Thanksgiving ends

  • The comparison poster shows a Nov 27 expiry, so the disable state persists past the final inclusive activation day.

View artifacts

T-Rex Ran code and verified through T-Rex

Fix in Codex Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants