UDS-1897: feat(component-carousel): added functionality to check screen width for modified perview value #1406
Conversation
…or modified perview value Added functionality to check screen width for modified perview items value and to decide whether to show action buttons. UDS-1897
| let perViewValue; | ||
| switch (perView) { | ||
| case "3": | ||
| perViewValue = screenWidth > 1024 ? 3 : screenWidth > 768 ? 2 : 1; |
There was a problem hiding this comment.
Breaking this out to separate if, else statements will fix the build issue. We usually try to avoid nested ternaries.
Ex: if (screenWidth > 1024) { perViewValue = 3; } else if (screenWidth > 768) { perViewValue = 2; } else { perViewValue = 1; }
There was a problem hiding this comment.
Solved this build issue
| case "2": | ||
| perViewValue = screenWidth < 768 ? 1 : 2; | ||
| break; | ||
| case "1": |
There was a problem hiding this comment.
This can be removed as the default will take care of everything not mentioned above
There was a problem hiding this comment.
This if statement is necessary to render only one card on mobile when the carousel previously displayed two items per view.
There was a problem hiding this comment.
Sorry I think I commented on the wrong line, I meant the case "1" can be removed since the default is right under it and it's just gonna default if its not "2" or "3"
There was a problem hiding this comment.
I misunderstood. I thought you were referring to the second ternary case.
|
Just another suggestion but not required, assigning the 1024 and 768 numbers to constants at the top of the file will make it a bit cleaner to read. |
Breaking ternarie to separate if, else statement UDS-1897
|
Good suggestion, I added the constants |
There was a problem hiding this comment.
Is this duplicating the code that's already set up to do this check? https://github.com/ASU/asu-unity-stack/blob/UDS-1897/packages/component-carousel/src/core/components/BaseCarousel/glide/glide.setup.js#L10-L40 ?
Hi Michael, |
|
@juanmitriatti Let's hold on this PR until we hear back from Kathy about the other Carousel issues. |
Added functionality to check screen width for modified perview items value and to decide whether to show action buttons.
UDS-1897
Description
Links
FOR APPROVERS