fix(chat): make jumbo emotes fit the column and sit on the name's line - #31
Merged
Conversation
Verified against live chat (kaicenat, ~28k viewers) and the flat multiplier does not fit. In a 370px source at emotescale=3, an eleven-emote message rendered 77px per emote, wrapped to four lines and stood 275px tall: 35% of the overlay for one message. A single-emote reaction at the same setting is 77px and looks right. So divide the extra size among the art rather than applying it to each piece: N emotes get 1 + (scale - 1) / N. A row's added width then stays near (scale - 1) emote widths whatever the count, so one emote still lands at full size and spam degrades instead of taking the column. Six emotes at 3x now measure 34px each, 4% of the overlay, down from ~20%. Kept as a calc() over --hb-emote-boost rather than a number resolved in JS, so the boost stays overridable from OBS Custom CSS. Also drop the -0.25rem block tuck on jumbo rows. It exists so a 1.6em emote does not push a ~1.35em line apart; on a jumbo row the art is taller than the line box, so the fixed tuck let it bleed over the rows above and below. Measured: a one-emote row was 69px tall around 77px of art. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
WalkthroughJumbo scaling now distributes the extra multiplier across emotes in emote-only chat rows. The resolver counts emotes and cheermotes, rendering uses a CSS ChangesJumbo emote scaling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Centring an emote on the text line is right at chat size and wrong once the art is several times the line height: the name ends up floating against the middle of a tall block with no shared edge. Measured on a 77px emote at emotescale=3, in a real row on live chat: middle left the emote's lower edge 31px below the name, bottom closed that to 2px. baseline lands in the same place visually but adds about 5px of descender leading under every jumbo row, which is height an overlay cannot spare, so bottom it is. HbRoot writes --hb-emote-align, and only hb-message[data-emote-only] reads it, so the value is middle whenever emotescale is off and a default overlay renders exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/lib/emotes/resolve.test.ts (1)
104-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the invariant assertion unconditional.
The
ifskips all assertions whenisEmoteOnlyreturnsfalse. The test can pass if the predicate rejects every row. Assert thatisEmoteOnly(parts)istruebefore checkingemoteOnlyCount(parts).Proposed test fix
- if (isEmoteOnly(parts)) { - expect(emoteOnlyCount(parts)).toBeGreaterThan(0); - } + expect(isEmoteOnly(parts)).toBe(true); + expect(emoteOnlyCount(parts)).toBeGreaterThan(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 `@apps/web/src/lib/emotes/resolve.test.ts` around lines 104 - 111, Make the test covering isEmoteOnly and emoteOnlyCount assert isEmoteOnly(parts) unconditionally for each defined emote-only row, then assert emoteOnlyCount(parts) is greater than zero; remove the conditional guard so the test cannot pass without validating the predicate.
🤖 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 `@apps/web/src/routes/docs.tsx`:
- Line 69: Update the documentation body for the multiplier setting to define N
as the total art count, explicitly stating that it includes one cheermote when
hasCheermote is true, even if message.parts is empty. Keep the existing
multiplier formula and behavior description unchanged.
---
Nitpick comments:
In `@apps/web/src/lib/emotes/resolve.test.ts`:
- Around line 104-111: Make the test covering isEmoteOnly and emoteOnlyCount
assert isEmoteOnly(parts) unconditionally for each defined emote-only row, then
assert emoteOnlyCount(parts) is greater than zero; remove the conditional guard
so the test cannot pass without validating the predicate.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b64e861-8560-459a-8046-18d8f0eebc50
📒 Files selected for processing (6)
CLAUDE.mdapps/web/src/components/chat/chat-message.tsxapps/web/src/components/chat/overlay.cssapps/web/src/lib/emotes/resolve.test.tsapps/web/src/lib/emotes/resolve.tsapps/web/src/routes/docs.tsx
| name: "emotescale", | ||
| values: "1 to 4, half steps", | ||
| body: "Grows emotes on messages that are nothing but emotes. One word alongside the emote and that message renders at normal size, so a wall of PogChamp stands out and a sentence does not. Any value between the half steps snaps to the nearest one. Cheermote art follows the same multiplier; badges, pronoun pills, avatars and text do not. This counts toward the source art the overlay requests the same way size does, so raising it past 2x pulls larger files from the emote services and costs more bandwidth. Default 1.", | ||
| body: "Grows emotes on messages that are nothing but emotes. One word alongside the emote and that message renders at normal size, so a wall of PogChamp stands out and a sentence does not. Any value between the half steps snaps to the nearest one. The multiplier is shared out across the emotes in the message: a single emote gets the full value, and a message of N gets 1 + (value - 1) / N. Spam therefore grows a little and one reaction grows a lot, which keeps an eleven-emote message from filling a third of the source. Cheermote art follows the same multiplier; badges, pronoun pills, avatars and text do not. This counts toward the source art the overlay requests the same way size does, so raising it past 2x pulls larger files from the emote services and costs more bandwidth. Default 1.", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Define N as the total art count.
emoteOnlyCount includes one cheermote when hasCheermote is true, even when message.parts is empty. The documentation says the multiplier is shared across emotes, then states that cheermotes use the same multiplier. This leaves the denominator ambiguous for cheer messages. State that N includes one cheermote when present.
Proposed documentation fix
- The multiplier is shared out across the emotes in the message: a single emote gets the full value, and a message of N gets 1 + (value - 1) / N.
+ The multiplier is shared out across the emote art in the message, including one cheermote when present: a single art element gets the full value, and a message with N art elements gets 1 + (value - 1) / N.📝 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.
Suggested change
| body: "Grows emotes on messages that are nothing but emotes. One word alongside the emote and that message renders at normal size, so a wall of PogChamp stands out and a sentence does not. Any value between the half steps snaps to the nearest one. The multiplier is shared out across the emotes in the message: a single emote gets the full value, and a message of N gets 1 + (value - 1) / N. Spam therefore grows a little and one reaction grows a lot, which keeps an eleven-emote message from filling a third of the source. Cheermote art follows the same multiplier; badges, pronoun pills, avatars and text do not. This counts toward the source art the overlay requests the same way size does, so raising it past 2x pulls larger files from the emote services and costs more bandwidth. Default 1.", | |
| body: "Grows emotes on messages that are nothing but emotes. One word alongside the emote and that message renders at normal size, so a wall of PogChamp stands out and a sentence does not. Any value between the half steps snaps to the nearest one. The multiplier is shared out across the emote art in the message, including one cheermote when present: a single art element gets the full value, and a message with N art elements gets 1 + (value - 1) / N. Spam therefore grows a little and one reaction grows a lot, which keeps an eleven-emote message from filling a third of the source. Cheermote art follows the same multiplier; badges, pronoun pills, avatars and text do not. This counts toward the source art the overlay requests the same way size does, so raising it past 2x pulls larger files from the emote services and costs more bandwidth. Default 1.", |
🤖 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 `@apps/web/src/routes/docs.tsx` at line 69, Update the documentation body for
the multiplier setting to define N as the total art count, explicitly stating
that it includes one cheermote when hasCheermote is true, even if message.parts
is empty. Keep the existing multiplier formula and behavior description
unchanged.
nathanialhenniges
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #30, from watching it run against live chat on
kaicenat(~28k viewers) in a 370px source, which is a realistic OBS width.Two problems, both measured rather than guessed.
1. A spam row ate the column
The flat multiplier treats a deliberate one-emote reaction and an eleven-emote wall identically, and those are not the same message.
Fix: divide the extra size among the art. N emotes get
1 + (scale - 1) / N.The useful property: total row width is about
base x (N + scale - 1), so a row is only ever(scale - 1)emote widths wider than at 1x, however many were spammed. One emote keeps the full multiplier; spam degrades toward normal.Measured after, same channel and settings: six emotes at 3x render 34px each, 4% of the overlay, down from roughly 20%.
Kept as a
calc()over--hb-emote-boostrather than a number resolved in JS, so the boost stays overridable from OBS Custom CSS.2. The name floated in the middle of the emote
Centring an image on the text line is correct at chat size and wrong once the art is several times the line height: the name ends up against the middle of a tall block with no shared edge. Browsers default inline images to
baseline; Twitch, YouTube and Slack override tomiddlefor chat emotes, which is the right call at 1.6em and stops being right at 4.8em.A/B/C on a real row with a 77px emote:
middle(before)baselinebottomFix:
bottom, which wins on alignment and costs no extra height.HbRootwrites--hb-emote-alignand onlyhb-message[data-emote-only]reads it, so the value ismiddlewheneveremotescaleis off and a default overlay renders byte-identically to before.Also
Dropped the
-0.25remblock tuck on jumbo rows. It exists so a 1.6em emote does not push a ~1.35em line apart; on a jumbo row the art is taller than the line box, so a fixed tuck let it bleed over the neighbouring rows. A one-emote row measured 69px tall around 77px of art.Verification
bun test: 134 pass, including newemoteOnlyCountcoverage and a guard that it never returns 0 for a rowisEmoteOnlyaccepts (that count is a divisor)check-typesandbiome checkclean🤖 Generated with Claude Code