Skip to content

Conversation

@Eetwalt
Copy link
Collaborator

@Eetwalt Eetwalt commented Feb 4, 2026

Summary

Resolves #6951

Problem:
Carousels can be initialized while hidden (e.g. inside general-collapsible or ContentSwitch/toggle-area), so initial width measurements are 0 and the controls/scroll behavior don’t work when the content is later revealed.

Fix:
Each carousel now attaches a ResizeObserver to its content and schedules a refresh (handleResize) when its size changes, so it self-heals automatically once it becomes visible.

Also made carousel fader color configurable with a fallback so it's more flexible usable inside components like collapsibles:
image

How did you test this change?

dev tools on https://liquipedia.net/leagueoflegends/Module:Sandbox/Eetwalt

screenrecording-2026-02-04_15-37-12.mp4

@Eetwalt Eetwalt requested review from a team February 4, 2026 12:33
@Eetwalt Eetwalt added the javascript Changes to JavaScript files label Feb 4, 2026
Copilot AI review requested due to automatic review settings February 4, 2026 12:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where carousels inside collapsible sections were non-functional because they initialized while hidden (display:none), causing width and controls to be computed as 0.

Changes:

  • Added a scheduleLayoutUpdate function that triggers carousel resize handling after collapsible state changes
  • Integrated layout updates into all collapsible toggle event handlers

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Eetwalt Eetwalt requested a review from hjpalpha February 4, 2026 13:38
Comment on lines 97 to 103
if ( typeof ResizeObserver === 'function' ) {
const ro = new ResizeObserver( () => {
liquipedia.carousel.scheduleRefresh();
} );
ro.observe( carouselData.content );
carouselData.resizeObserver = ro;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need the outer if here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added it as safety measure for older browser. ResizeObserver is Baseline Widely available, but still wanted to be safe

Copy link
Collaborator

@Rathoz Rathoz Feb 4, 2026

Choose a reason for hiding this comment

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

It's widely enough available. It's very close to the levels where webp is today, which we been using for a year or two already.

https://caniuse.com/resizeobserver

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's widely enough available.

Meaning we should drop the outer if? Or good to merge as is

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think Rath means that it should be dropped as it is safe enough to assume that the check is a tautology

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Eetwalt Eetwalt changed the title fix: carousel doesn't work inside collapsible fix: carousel doesn't work inside collapsible/content switch Feb 4, 2026
@Rathoz
Copy link
Collaborator

Rathoz commented Feb 9, 2026

Change the lint to use stylelint-config-wikimedia/support-modern instead

@Eetwalt
Copy link
Collaborator Author

Eetwalt commented Feb 9, 2026

Change the lint to use stylelint-config-wikimedia/support-modern instead

Thats css linting? Added "compat/compat": "off", to .eslintrc.json to make the linter happy. This fine?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Changes to JavaScript files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Carousel does not initialize properly when placed inside collapsible

4 participants