fix(gazzodown): render timestamp inside strikethrough text#39256
fix(gazzodown): render timestamp inside strikethrough text#39256Khizarshah01 wants to merge 1 commit intoRocketChat:developfrom
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughAdds missing TIMESTAMP rendering support to the StrikeSpan component. Imports the Timestamp component and extends the conditional rendering logic to handle Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/gazzodown/src/elements/StrikeSpan.tsx (1)
35-36: Minor inconsistency with EMOJI handling.The PR notes that timestamps "appear as formatted badges, so the
<del>strikethrough line is not visually applied to them (same behavior as emojis inside strikethrough)." However,EMOJIis intentionally excluded from the<del>wrapper (handled on line 40), whileTIMESTAMPis being wrapped here.This works correctly and achieves the goal, but for consistency with EMOJI, you could move
TIMESTAMPout of this condition. That said, wrapping in<del>is semantically correct even if not visually rendered, so the current approach is also reasonable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/StrikeSpan.tsx` around lines 35 - 36, The condition that excludes block types from the <del> wrapper currently lists TIMESTAMP alongside INLINE_CODE; to match the EMOJI handling move TIMESTAMP out of that exclusion so TIMESTAMP is not treated as a non-del-wrapped inline here. Locate the check on block.type in StrikeSpan (the expression containing INLINE_CODE and TIMESTAMP) and remove TIMESTAMP from it, ensuring TIMESTAMP is handled the same way EMOJI is (the separate logic referenced around the EMOJI handling near the other conditional on line ~40).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/gazzodown/src/elements/StrikeSpan.tsx`:
- Around line 35-36: The condition that excludes block types from the <del>
wrapper currently lists TIMESTAMP alongside INLINE_CODE; to match the EMOJI
handling move TIMESTAMP out of that exclusion so TIMESTAMP is not treated as a
non-del-wrapped inline here. Locate the check on block.type in StrikeSpan (the
expression containing INLINE_CODE and TIMESTAMP) and remove TIMESTAMP from it,
ensuring TIMESTAMP is handled the same way EMOJI is (the separate logic
referenced around the EMOJI handling near the other conditional on line ~40).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/gazzodown/src/elements/StrikeSpan.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/gazzodown/src/elements/StrikeSpan.tsx
🔇 Additional comments (1)
packages/gazzodown/src/elements/StrikeSpan.tsx (1)
8-8: LGTM!Import is correctly added and follows the existing pattern for sibling element components.
Proposed changes (including videos or screenshots)
Timestamps inside strikethrough text were not rendering, they silently vanished. The parser grammar explicitly supports timestamps inside strikethrough (
StrikethroughContentincludesTimestampRules), andStrikeSpantype already includedMessageParser.Timestamp, but the render switch had no case for it.This PR adds the missing TIMESTAMP case to
StrikeSpan.Before
Screencast.from.2026-03-03.00-44-25.mp4
After (Fix)
Screencast.from.2026-03-03.00-59-37.mp4
Issue(s)
Fixes #39255
Summary by CodeRabbit