Skip to content

Fix: Fixes incorrect marquee width calculations #164

Merged
pravton merged 8 commits into
mainfrom
fix/marquee-width-calculation
Oct 30, 2025
Merged

Fix: Fixes incorrect marquee width calculations #164
pravton merged 8 commits into
mainfrom
fix/marquee-width-calculation

Conversation

@pravton
Copy link
Copy Markdown
Contributor

@pravton pravton commented Oct 27, 2025

Description

This PR fixes the width calculation issue we faced when children don't have a width on render.

For example, if the children use flex or the child elements don't have constrained widths, the marquee would throw an error. This happens when an image is loading during render and the parent element resolves to 0—the needed amount would be Infinity to fill the container, causing Invalid array length error when rendering the MarqueeItems.

Solution

I have added ResizeObserver to watch changes on the container and this would re-render the component with updated calculation which would typically solves this issue.

All modern browsers support ResizeObserver, but I've kept the resize event listener for legacy browser support. Let me know if you think ResizeObserver alone is sufficient—we can probably remove the resize event listener.

I've tested with flex children and images without specified widths—the marquee calculates the width correctly in all cases.

Resolves #163

Additional Notes

I've also updated the placeholder URLs from upsplash.io to placehold.co since the Unsplash URLs were resolving super slowly.

@marlonmarcello
Copy link
Copy Markdown
Member

All modern browsers support ResizeObserver, but I've kept the resize event listener for legacy browser support. Let me know if you think ResizeObserver alone is sufficient—we can probably remove the resize event listener.

Let's remove that old one, ResizeObserver has been around for a while and supported in Safari since v13 even.
That said, let's make this a major bump though as we are essentially not supporting those really old browser versions anymore.

@marlonmarcello
Copy link
Copy Markdown
Member

Actually, this repo is setup with changesets @pravton so please use that.
Simply run npm run changeset

Copy link
Copy Markdown
Member

@andrewrubin andrewrubin left a comment

Choose a reason for hiding this comment

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

Code is looking great @pravton ! I agree with Marlon, let's use changeset to make this a major bump instead of minor (3.0.0)

@pravton pravton force-pushed the fix/marquee-width-calculation branch from fbbe724 to 52c1da9 Compare October 28, 2025 18:16
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Oct 28, 2025

🦋 Changeset detected

Latest commit: 94cd684

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@wethegit/react-marquee Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pravton
Copy link
Copy Markdown
Contributor Author

pravton commented Oct 28, 2025

Code is looking great @pravton ! I agree with Marlon, let's use changeset to make this a major bump instead of minor (3.0.0)

Thanks Andrew! Updated and used changeset for major version bump.

FYI: I have also rebased this branch since I was merging some Dependabot PRs.

@pravton pravton requested a review from andrewrubin October 28, 2025 18:23
Comment thread src/lib/marquee.tsx Outdated
Comment thread src/lib/marquee.tsx
Comment thread src/lib/marquee.tsx Outdated
@pravton pravton requested a review from andrewrubin October 28, 2025 23:12
andrewrubin
andrewrubin previously approved these changes Oct 29, 2025
Copy link
Copy Markdown
Member

@andrewrubin andrewrubin left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread src/lib/marquee.tsx
Comment thread src/lib/marquee.tsx Outdated
marlonmarcello
marlonmarcello previously approved these changes Oct 29, 2025
Copy link
Copy Markdown
Member

@marlonmarcello marlonmarcello left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @pravton

Co-authored-by: Marlon U. Marcello <marlon.marcello@gmail.com>
Signed-off-by: Clinton <80900245+pravton@users.noreply.github.com>
@pravton pravton dismissed stale reviews from marlonmarcello and andrewrubin via 94cd684 October 29, 2025 23:15
@pravton
Copy link
Copy Markdown
Contributor Author

pravton commented Oct 29, 2025

Thanks @marlonmarcello, committed your suggestion and need another ✅ when you get a chance!

@pravton pravton requested a review from andrewrubin October 29, 2025 23:17
@pravton pravton merged commit d115f85 into main Oct 30, 2025
7 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 30, 2025
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.

Marquee does not calculate the width properly on some instance - needs a reactor

3 participants