Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Carousel: Visual bugs when loading carousel with responsiveCarouselOptions #5166

Closed
mzaninovic555 opened this issue Oct 26, 2023 · 10 comments · Fixed by #5840
Closed

Carousel: Visual bugs when loading carousel with responsiveCarouselOptions #5166

mzaninovic555 opened this issue Oct 26, 2023 · 10 comments · Fixed by #5840
Assignees
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@mzaninovic555
Copy link

Describe the bug

When loading the component for the first time, the page marker will have the incorrect number of pages, and moving to another page makes the elements disappear. Also, it skips the wrong amount of elements, instead of the defined numScroll, it will scroll by the number of elements in a page.

The problem disappears if the viewport width changes (by resizing the window or opening inspect window).

Reproducer

https://codesandbox.io/s/primereact-test-forked-2s7n95

PrimeReact version

10.0.7

React version

18.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

No response

Steps to reproduce the behavior

  1. Create a Carousel with responsiveOptions prop - the element loads with wrong page count
  2. Try moving to a different carousel page - the elements disappear
  3. Resize window - the elements reapper and page count is fixed

Expected behavior

Carousel should load the correct page count, and elements should move in the defined numScroll amount and elements should not disappear.

@mzaninovic555 mzaninovic555 added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Oct 26, 2023
@melloware
Copy link
Member

I think your issue is similar to this one: #3985

@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Oct 26, 2023
@ArsenicBismuth
Copy link

ArsenicBismuth commented Dec 23, 2023

I got this issue too (using Next.Js), but later on found out that it's only an issue on dev build. Running on prod build and everything works well.

@cagataycivici
Copy link
Member

Could be related to StrictMode?

@ArsenicBismuth
Copy link

Nope. In my case, the bug still appears in dev mode with both strict and non-strict mode.

@ArsenicBismuth
Copy link

Update: Turns out the bug is still there, but not in every scenario.

  • In dev: The bug WILL always come in any scenario as long as your screen size is under the breakpoint.
  • In prod: The bug will come only if you start the website at a screen size under the breakpoint. But if you start at full screen, and later resize your window down, it will work perfectly.

@arnodemer
Copy link
Contributor

@melloware, I am able to reproduce same kind of issue using primereact 10.3.1 (we don't use React.StricMode).
My carousel use prop. responsiveOptions which configure number of visible carousel items to, let said, 5 items from current window.innerWidth, but I intentionnaly set carousel prop. numVisible=2 (note: 2 lower than 5).
What happens : At first rendering, the 5 visible items are rendered correctly, but when I move mouse over carousel, the last 3 ones disappears.
Debugging carousel.js show me that calculatePosition() is called too early, at first call: responsiveOptions.current is undefined.
Meaning that numVisibleState is not updated from window.innerWidth and responsiveOptions values (5 items), but keep it initial value set from carousel props.numVisible (2 items).
As calculatePosition() is also responsible to update numScrollState, page, totalShiftedItems , this should explain issue described by @mzaninovic555 here. (I can see same behavior if I use scroll right/left button without resizing browser).

To make the 5 items appearing again, I need to resize browser manually -> calculatePosition() is called again thanks to useResizeListener().

@melloware
Copy link
Member

PR is welcome @arnodemer

@enzolry
Copy link

enzolry commented Jan 16, 2024

Same problem. I ended up abandoning the responsiveOptions thing and make my own responsive implementation based on a custom hook returning preconfigured breakpoints on page resize. Since the Carousel does not refresh when updating the values of numVisible and numScroll props, I set a "reloading" state to true for 1 refresh just to return null and force component to refresh.

const { breakpoint } = useResponsive();

const [reloading, setReloading] = useState(false);

useEffect(() => {
  setReloading(true);
  setTimeout(() => setReloading(false), 0);
}, [breakpoint]);

if (reloading) return null;

return <Carousel />

@baonk
Copy link

baonk commented Jan 25, 2024

@melloware on createStyle function, after check props.responsiveOptions please add calculatePosition(); then it will fix this bug.

                  if (props.responsiveOptions) {
			const comparator = ObjectUtils.localeComparator((context && context.locale) || PrimeReact.locale);

			responsiveOptions.current = [...props.responsiveOptions];
			responsiveOptions.current.sort((data1, data2) => {
				const value1 = data1.breakpoint;
				const value2 = data2.breakpoint;

				return ObjectUtils.sort(
					value1,
					value2,
					-1,
					comparator,
					(context && context.nullSortOrder) || PrimeReact.nullSortOrder
				);
			});

			for (let i = 0; i < responsiveOptions.current.length; i++) {
				let res = responsiveOptions.current[i];

				innerHTML += `
                @media screen and (max-width: ${res.breakpoint}) {
                    .p-carousel[${attributeSelector.current}] .p-carousel-item {
                        flex: 1 0 ${100 / res.numVisible}%
                    }
                }
            `;
			}

			calculatePosition(); //Add this line
		} 

@melloware melloware added this to the 10.4.0 milestone Jan 25, 2024
melloware added a commit to melloware/primereact that referenced this issue Jan 25, 2024
@melloware
Copy link
Member

PR submitted @baonk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants