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

feat: expanded functionality for EsCarousel #1558

Merged

Conversation

ericdouglaspratt
Copy link
Collaborator

@ericdouglaspratt ericdouglaspratt commented Oct 31, 2024

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

  • EsCarousel now takes numVisible, numScroll, and breakpoints props to customize the number of slides shown per breakpoint and number of slides that scroll on each paging event
  • The breakpoints is set up in a way that makes sense with our breakpoints and how we use them in components like EsCol, for example, rather than conforming to PrimeVue Carousel's odd reversed max-width approach
  • Renamed autoscroll prop to autoPlay and added autoPlayInterval prop
  • Renamed dots prop to showDots and added showArrows prop
  • Added circular prop
    • Fixed an SSR issue with circular behavior where the wrong item would be briefly displayed as the first item on page load, then hydration would correct it
  • ESDS is now exporting types (for EsCarousel's breakpoints prop)
  • On the docs page, since I'm very nervous about relying on a third-party CDN for example images, switched to using images from Storyblok prod we'll likely never delete and used a more generic card design instead of something with such specific styling
  • Known issue: responsive carousels briefly show mobile amount of dots on desktop, until hydration
    • this is preexisting and since none of the homepage carousels have this issue, i left solving it out of scope for now

πŸ₯Ό Testing

  • Tested locally with ESDS docs site
  • Also tested on es-cms-pages homepage with the logo carousel and the hero carousel

🧐 Feedback Requested / Focus Areas

  • Overall

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.
  • I have documented testing approach

Copy link

swarmia bot commented Oct 31, 2024

@ericdouglaspratt ericdouglaspratt changed the title feat: EsCarousel custom responsiveness and more props exposed feat: expanded functionality for EsCarousel Nov 1, 2024
@hroth1994
Copy link
Contributor

πŸ‘€

Copy link
Contributor

@hroth1994 hroth1994 left a comment

Choose a reason for hiding this comment

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

Nice work on this!
Just left one comment for fun:

es-ds-components/components/es-carousel.vue Outdated Show resolved Hide resolved
@ericdouglaspratt ericdouglaspratt merged commit 75590a0 into esds-3.0-vue3-primevue Nov 1, 2024
1 check passed
@ericdouglaspratt ericdouglaspratt deleted the ced-1900-es-carousel-enhancements branch November 1, 2024 20:11
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.

2 participants