feat(website): remove cls on typewritter effect - #1293
Open
jonatankruszewski wants to merge 1 commit into
Open
Conversation
|
@jonatankruszewski is attempting to deploy a commit to the ealush's projects Team on Vercel. A member of the Team first needs to authorize it. |
Review Summary by QodoFix typewriter layout shifts with invisible sizing layer
WalkthroughsDescription• Fixes layout shifts in typewriter component by reserving space upfront • Implements invisible sizing layer with all phrase variants • Overlays visible animated text on reserved space grid • Adds responsive breakpoint handling for mobile devices Diagramflowchart LR
A["Typewriter Component"] -->|receives reservePairs| B["Generate Reserve Variants"]
B -->|all phrase combinations| C["Invisible Sizer Layer"]
C -->|reserves space| D["Grid Layout"]
D -->|overlays| E["Visible Animated Text"]
E -->|maintains animation| F["No Layout Shift"]
File Changes1. website/src/components/Typewriter/index.js
|
📝 WalkthroughWalkthroughAdded a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Code Review by Qodo
1. Indented value line
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a sizer mechanism to the Typewriter component to prevent layout shifts during animations by pre-calculating the maximum required space. Key changes include the addition of a reservePairs prop, a grid-based layout for overlapping hidden and visible text, and responsive breakpoint handling. Review feedback suggests memoizing the variant calculations to optimize performance during frequent re-renders and refining the breakpoint implementation by moving space characters into CSS to ensure consistent alignment across desktop and mobile views.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
website/src/components/Typewriter/index.js (2)
132-138: Optional: memoizereserveVariants.
reserveVariantsis rebuilt on every render, and the component re-renders on every typing tick (~30–70ms). The cost is small for today’sTYPEWRITER_DATA, but wrapping inuseMemokeyed onreservePairs/prefix/valuesmakes the work proportional to actual prop changes and also lets React skip reconciling the sizer subtree across ticks when memoized further down.♻️ Proposed tweak
- const reserveVariants = (reservePairs ?? [[prefix, values]]).flatMap( - ([reservePrefix, reserveValues]) => - (reserveValues.length ? reserveValues : ['']).map(reserveValue => ({ - prefix: reservePrefix, - value: reserveValue, - })), - ); + const reserveVariants = React.useMemo( + () => + (reservePairs ?? [[prefix, values]]).flatMap( + ([reservePrefix, reserveValues]) => + (reserveValues.length ? reserveValues : ['']).map(reserveValue => ({ + prefix: reservePrefix, + value: reserveValue, + })), + ), + [reservePairs, prefix, values], + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/Typewriter/index.js` around lines 132 - 138, Wrap the computation of reserveVariants in React's useMemo so it only rebuilds when inputs change: memoize the expression that uses reservePairs, prefix and values (the current flatMap producing objects with prefix/value) by replacing the direct declaration of reserveVariants with a useMemo that lists reservePairs, prefix, and values as dependencies; this reduces work on each typing tick and lets downstream memoization of the sizer subtree be effective.
141-164: Minor cleanups: deadclsxcondition and variable shadowing.Two small readability items in the render tree:
- Line 161:
clsx(styles.cursor, { [styles.blinking]: false })always resolves to juststyles.cursor— the prefix cursor never blinks, so the conditional entry is dead.- Line 143: destructuring
{ prefix, value }inside the map shadows the outerprefixprop. It works today because the map body only needs the inner values, but it’s easy to trip over in future edits.♻️ Proposed tweak
- <span className={styles.sizer} aria-hidden="true"> - {reserveVariants.map(({ prefix, value }, index) => ( - <span - className={styles.sizerVariant} - key={`${prefix}-${value}-${index}`} - > - {prefix} - <span className={styles.cursor}>|</span> - <span className={styles.breakPoint}> </span> - <span className={highlightClassName}> - {value} - <span className={styles.cursor}>|</span> - </span> - </span> - ))} - </span> + <span className={styles.sizer} aria-hidden="true"> + {reserveVariants.map( + ({ prefix: variantPrefix, value: variantValue }, index) => ( + <span + className={styles.sizerVariant} + key={`${variantPrefix}-${variantValue}-${index}`} + > + {variantPrefix} + <span className={styles.cursor}>|</span> + <span className={styles.breakPoint}> </span> + <span className={highlightClassName}> + {variantValue} + <span className={styles.cursor}>|</span> + </span> + </span> + ), + )} + </span> <span className={styles.visibleText}> {displayedPrefix} {showPrefixCursor && ( - <span className={clsx(styles.cursor, { [styles.blinking]: false })}> - | - </span> + <span className={styles.cursor}>|</span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/Typewriter/index.js` around lines 141 - 164, The render contains two small issues to fix: in the reserveVariants.map callback avoid shadowing the outer prefix prop by renaming the inner destructured variables (e.g., change "{ prefix, value }" to "{ prefix: variantPrefix, value: variantValue }" or similar) so the outer prefix prop remains unshadowed; and simplify the dead clsx call used for the prefix cursor by removing the always-false conditional object (replace "clsx(styles.cursor, { [styles.blinking]: false })" with just styles.cursor or a clsx call that only includes styles.blinking when a real condition is present) so the cursor class isn’t cluttered with a no-op entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/src/components/Typewriter/index.js`:
- Around line 132-138: Wrap the computation of reserveVariants in React's
useMemo so it only rebuilds when inputs change: memoize the expression that uses
reservePairs, prefix and values (the current flatMap producing objects with
prefix/value) by replacing the direct declaration of reserveVariants with a
useMemo that lists reservePairs, prefix, and values as dependencies; this
reduces work on each typing tick and lets downstream memoization of the sizer
subtree be effective.
- Around line 141-164: The render contains two small issues to fix: in the
reserveVariants.map callback avoid shadowing the outer prefix prop by renaming
the inner destructured variables (e.g., change "{ prefix, value }" to "{ prefix:
variantPrefix, value: variantValue }" or similar) so the outer prefix prop
remains unshadowed; and simplify the dead clsx call used for the prefix cursor
by removing the always-false conditional object (replace "clsx(styles.cursor, {
[styles.blinking]: false })" with just styles.cursor or a clsx call that only
includes styles.blinking when a real condition is present) so the cursor class
isn’t cluttered with a no-op entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9473cec-707a-45aa-8c5c-be0195a42d9c
📒 Files selected for processing (3)
website/src/components/Typewriter/index.jswebsite/src/components/Typewriter/styles.module.csswebsite/src/pages/index.js
jonatankruszewski
force-pushed
the
remove-cls-docs
branch
from
April 16, 2026 19:33
22450b9 to
f693f41
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/src/components/Typewriter/index.js (1)
132-142: Minor: brittle assumption onreservePairsshape.
reserveValues.length(Line 136) will throw if a caller passes a pair whose second element isundefined(e.g.[[someLabel]]). For the current homepage usage withTYPEWRITER_DATAthis should be safe, but a tiny defensive default keeps this robust if the dataset shape ever drifts.🛡️ Suggested defensive handling
- (reservePairs ?? [[prefix, values]]).flatMap( - ([reservePrefix, reserveValues]) => - (reserveValues.length ? reserveValues : ['']).map(reserveValue => ({ - prefix: reservePrefix, - value: reserveValue, - })), - ), + (reservePairs ?? [[prefix, values]]).flatMap( + ([reservePrefix = prefix, reserveValues = []]) => + (reserveValues.length ? reserveValues : ['']).map(reserveValue => ({ + prefix: reservePrefix, + value: reserveValue, + })), + ),This also aligns the behavior with the summary's description of falling back to the component's
prefixwhenreservePrefixis missing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/Typewriter/index.js` around lines 132 - 142, The code assumes each pair in reservePairs has a second element array and accesses reserveValues.length which can throw; update the useMemo mapping in reserveVariants to defensively coerce reserveValues to an array (e.g., const rv = Array.isArray(reserveValues) ? reserveValues : ['']) and use rv.length when deciding to map, and also fall back to the component-level prefix when reservePrefix is falsy (use prefix). Adjust references to reservePairs, reservePrefix, reserveValues, prefix, and values inside the reserveVariants computation accordingly so undefined or malformed pairs don't throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/src/components/Typewriter/index.js`:
- Around line 132-142: The code assumes each pair in reservePairs has a second
element array and accesses reserveValues.length which can throw; update the
useMemo mapping in reserveVariants to defensively coerce reserveValues to an
array (e.g., const rv = Array.isArray(reserveValues) ? reserveValues : ['']) and
use rv.length when deciding to map, and also fall back to the component-level
prefix when reservePrefix is falsy (use prefix). Adjust references to
reservePairs, reservePrefix, reserveValues, prefix, and values inside the
reserveVariants computation accordingly so undefined or malformed pairs don't
throw.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b9a8ff2-13b4-4703-92ac-4ce5de31d988
📒 Files selected for processing (3)
website/src/components/Typewriter/index.jswebsite/src/components/Typewriter/styles.module.csswebsite/src/pages/index.js
✅ Files skipped from review due to trivial changes (1)
- website/src/pages/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- website/src/components/Typewriter/styles.module.css
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.
This fixes layout shifts caused by the homepage typewriter changing its rendered dimensions while phrases are typed or when a longer phrase is selected.
The
Typewritercomponent now renders an invisible sizing layer with the full reserved text variants and overlays the visible animated text in the same grid position. This lets the component reserve the required space up front while keeping the existing typing animation behavior.The homepage passes all available hero typewriter phrases to
TypewriterthroughreservePairs, so the hero title reserves enough space for any randomly selected phrase group.Summary by CodeRabbit
New Features
Style