Skip to content

Treat printText duration 0 as persistent and add test#531

Merged
tracygardner merged 1 commit into
mainfrom
codex/make-printtext-block-persist-text
Apr 13, 2026
Merged

Treat printText duration 0 as persistent and add test#531
tracygardner merged 1 commit into
mainfrom
codex/make-printtext-block-persist-text

Conversation

@tracygardner
Copy link
Copy Markdown
Contributor

@tracygardner tracygardner commented Apr 13, 2026

Motivation

  • Ensure printText treats a duration of 0 as “persistent” (no auto-fade) instead of scheduling a timeout to remove it.
  • Add a test to prevent regressions and verify persistent behavior.

Description

  • Change timeoutId from a const to let and initialize it to null in api/ui.js to represent no scheduled timeout.
  • Only call setTimeout(fadeOut, ...) when safeDuration > 0, so a duration of 0 will not schedule an automatic fade.
  • Guard the abort handler to only call clearTimeout when timeoutId !== null, avoiding unnecessary clearing when no timeout was scheduled.
  • Add a test should keep the control when duration is 0 in tests/printtext.test.js that asserts the background control still exists after a short delay.

Testing

  • Ran the print text test suite including the new should keep the control when duration is 0 test, and all tests passed.
  • Existing tests for text content and color behavior (should print text, should use white as the default text color, should use the specified text color, should remove the control after the duration expires) were run and remained green.

Codex Task

Summary by CodeRabbit

  • Bug Fixes

    • Fixed text fade-out behavior to only activate when a specific duration is configured, improving edge case handling
    • Improved timeout cleanup logic to prevent unintended text removal when duration is zero
  • Tests

    • Added test coverage for persistent text display when duration is zero

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 13, 2026

Deploying flockxr with  Cloudflare Pages  Cloudflare Pages

Latest commit: 508efb9
Status: ✅  Deploy successful!
Preview URL: https://463034e3.flockxr.pages.dev
Branch Preview URL: https://codex-make-printtext-block-p.flockxr.pages.dev

View logs

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f0e01486-d890-4975-b46e-78ebee2d2b1e

📥 Commits

Reviewing files that changed from the base of the PR and between b5cf821 and 508efb9.

📒 Files selected for processing (2)
  • api/ui.js
  • tests/printtext.test.js

📝 Walkthrough

Walkthrough

The printText function in api/ui.js now conditionally schedules the fade-out timeout only when safeDuration is greater than zero, preventing unnecessary timeout calls for persistent text. The timeoutId is initialized to null and only cleared during abort if a timeout was actually scheduled. A new test validates that text persists indefinitely when duration is set to zero.

Changes

Cohort / File(s) Summary
UI Function Enhancement
api/ui.js
Modified printText to conditionally schedule fadeOut timeout only when safeDuration > 0; updated timeoutId initialization to null and added guard clause for timeout cleanup to prevent invalid clearTimeout calls.
Test Coverage
tests/printtext.test.js
Added test case validating that text control persists when duration is 0, ensuring indefinite display behavior is properly covered.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 With zero duration's gentle touch,
The text won't fade and worry much,
A timeout waits when needed true,
But rests content when time is through! ⏰✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately and concisely describes the main changes: treating printText duration 0 as persistent and adding a corresponding test case.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/make-printtext-block-persist-text

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying flockdev with  Cloudflare Pages  Cloudflare Pages

Latest commit: 508efb9
Status: ✅  Deploy successful!
Preview URL: https://7e5fafc8.flockdev.pages.dev
Branch Preview URL: https://codex-make-printtext-block-p.flockdev.pages.dev

View logs

@tracygardner tracygardner merged commit 6928646 into main Apr 13, 2026
10 checks passed
@tracygardner tracygardner deleted the codex/make-printtext-block-persist-text branch April 13, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant