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

Xuanying #957 disable carousel auto advance #962

Merged
merged 5 commits into from
Apr 23, 2023

Conversation

SleeplessRegina
Copy link
Collaborator

Disable carousel auto advance, make the carousel only support manually advance both in TestimonialCarousel and BlogCarousel.
Set the custom intervals to null and remove interval from our type statement.

@SleeplessRegina SleeplessRegina requested review from PeterBreen and marlonkeating and removed request for PeterBreen April 11, 2023 00:42
@SleeplessRegina SleeplessRegina self-assigned this Apr 11, 2023
Copy link
Collaborator

@PeterBreen PeterBreen left a comment

Choose a reason for hiding this comment

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

It looks good, but please see the three line comments on instances of using interval as a prop that we may not need. But please do let me know if it is necessary :)

@@ -263,7 +263,7 @@ class CorporateHackathonController extends React.PureComponent<{||}, State> {
<div className="corporate-hackathon-carousel carousel-testimonial-root">
<TestimonialCarousel
category="partner-highlights"
interval={600000}
interval={null}
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 to use interval={null} here, as well as in the TestimonialCarousel component itself? Carousel shouldn't be taking any props so I'm hoping it's safe to remove, but please test and let me know?

@@ -409,7 +409,7 @@ class CorporateHackathonController extends React.PureComponent<{||}, State> {
<div className="corporate-hackathon-carousel carousel-testimonial-root">
<TestimonialCarousel
category="hackathon-highlights"
interval={600000}
interval={null}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question as above

@@ -363,7 +363,7 @@ class LandingController extends React.PureComponent<{||}, State> {
<h2 className="text-center LandingController-section-header">
What people are saying about DemocracyLab
</h2>
<TestimonialCarousel interval={15000} />
<TestimonialCarousel interval={null} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question as above

@SleeplessRegina
Copy link
Collaborator Author

It looks good, but please see the three line comments on instances of using interval as a prop that we may not need. But please do let me know if it is necessary :)

Yes, I removed the interval props of TestimonialCarousel then tested again and found it still looks good. Both the carousel won't automatically advance.

@marlonkeating marlonkeating linked an issue Apr 13, 2023 that may be closed by this pull request
Copy link
Collaborator

@PeterBreen PeterBreen 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 those changes; looks good, works exactly as expected 👍

@SleeplessRegina SleeplessRegina merged commit 98d3d2e into master Apr 23, 2023
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.

Disable carousel autoadvance
2 participants