-
Notifications
You must be signed in to change notification settings - Fork 168
Fix: testimonials slider with improved styling and layout #263
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Gagan202005 <gagansinghal2005@gmail.com>
Signed-off-by: Gagan202005 <gagansinghal2005@gmail.com>
There was a problem hiding this 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 pull request enhances the testimonials slider component with improved styling, layout, and user experience. The changes include modern card designs with gradient backgrounds, hover effects, and better typography, while also fixing layout consistency issues across the page.
Key Changes:
- Simplified testimonial display from two rows to a single row marquee
- Enhanced card styling with gradients, borders, hover effects, and decorative elements (avatars with rings, online indicators, quote icons)
- Improved container layout to match site-wide consistency with
max-w-7xlwidth and responsive padding
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| alt="" | ||
| src={proxiedAvatar} | ||
| /> | ||
| <div className="absolute -bottom-1 -right-1 w-4 h-4 bg-orange-400 rounded-full border-2 border-white"></div> |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The decorative orange dot element lacks semantic meaning or alternative text. For better accessibility, consider adding aria-hidden="true" to this div since it's purely decorative and doesn't convey meaningful information to screen reader users.
| <div className="absolute -bottom-1 -right-1 w-4 h-4 bg-orange-400 rounded-full border-2 border-white"></div> | |
| <div | |
| className="absolute -bottom-1 -right-1 w-4 h-4 bg-orange-400 rounded-full border-2 border-white" | |
| aria-hidden="true" | |
| ></div> |
| <svg className="w-10 h-10 text-orange-300/40" fill="currentColor" viewBox="0 0 24 24"> | ||
| <path d="M14.017 21v-7.391c0-5.704 3.731-9.57 8.983-10.609l.995 2.151c-2.432.917-3.995 3.638-3.995 5.849h4v10h-9.983zm-14.017 0v-7.391c0-5.704 3.748-9.57 9-10.609l.996 2.151c-2.433.917-3.996 3.638-3.996 5.849h3.983v10h-9.983z"/> | ||
| </svg> |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The decorative quote icon SVG should have aria-hidden="true" added to it. Since this is purely decorative and doesn't convey meaningful information beyond what's already in the content, it should be hidden from assistive technologies.
| <svg className="w-3 h-3 text-gray-500" viewBox="0 0 24 24" fill="currentColor"> | ||
| <path d="M18.244 2.25h3.308l-7.227 8.26 8.502 11.24H16.17l-5.214-6.817L4.99 21.75H1.68l7.73-8.835L1.254 2.25H8.08l4.713 6.231zm-1.161 17.52h1.833L7.084 4.126H5.117z"/> | ||
| </svg> |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The X (Twitter) icon SVG should have aria-hidden="true" added to it since it's decorative and the username information is already conveyed through the text. This improves accessibility by preventing screen readers from announcing redundant information.
| <div className="flex flex-col"> | ||
| <figcaption className="text-sm font-bold">{name}</figcaption> | ||
| <p className="text-xs font-medium ">{id}</p> | ||
| <a href={post} target="_blank" className="lg:mx-2 inline-block"> |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The anchor tag opens in a new tab with target="_blank" but is missing rel="noopener noreferrer" for security. This is a security best practice to prevent the new page from accessing the window.opener property and to avoid passing the referrer information.
| <a href={post} target="_blank" className="lg:mx-2 inline-block"> | |
| <a href={post} target="_blank" rel="noopener noreferrer" className="lg:mx-2 inline-block"> |
| <figcaption className="text-sm font-bold">{name}</figcaption> | ||
| <p className="text-xs font-medium ">{id}</p> | ||
| <a href={post} target="_blank" className="lg:mx-2 inline-block"> | ||
| <figure className="relative w-80 min-h-64 cursor-pointer overflow-visible rounded-2xl border-2 border-gray-200/50 hover:border-orange-500 hover:shadow-xl hover:shadow-orange-200/50 hover:scale-[1.05] p-5 bg-gradient-to-br from-white to-orange-50/30 backdrop-blur-sm shadow-lg transition-all duration-300 flex flex-col"> |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The figure element has overflow-visible set, which may cause the hover scale effect (hover:scale-[1.05]) and shadow (hover:shadow-xl) to be clipped by parent containers. Consider verifying that parent elements have appropriate overflow handling or adjust this to overflow-hidden if clipping occurs.
| <figure className="relative w-80 min-h-64 cursor-pointer overflow-visible rounded-2xl border-2 border-gray-200/50 hover:border-orange-500 hover:shadow-xl hover:shadow-orange-200/50 hover:scale-[1.05] p-5 bg-gradient-to-br from-white to-orange-50/30 backdrop-blur-sm shadow-lg transition-all duration-300 flex flex-col"> | |
| <figure className="relative w-80 min-h-64 cursor-pointer overflow-hidden rounded-2xl border-2 border-gray-200/50 hover:border-orange-500 hover:shadow-xl hover:shadow-orange-200/50 hover:scale-[1.05] p-5 bg-gradient-to-br from-white to-orange-50/30 backdrop-blur-sm shadow-lg transition-all duration-300 flex flex-col"> |
components/testimonials.tsx
Outdated
| <p className="text-center lg:text-left text-gray-600 text-base mb-8"> | ||
| Join thousands of developers who trust Keploy for their testing needs | ||
| </p> | ||
| <div className="relative flex mb-8 h-fix w-full flex-col items-center justify-center overflow-hidden rounded-2xl border border-gray-200/50 bg-gradient-to-br from-orange-50/30 via-white to-orange-50/20 p-8 shadow-inner"> |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class h-fix appears to be a typo and is not a valid Tailwind CSS class. This should likely be h-full or a specific height value like h-[700px] (as it was in the original code). This will cause the container to not render with the intended height.
| <div className="relative flex mb-8 h-fix w-full flex-col items-center justify-center overflow-hidden rounded-2xl border border-gray-200/50 bg-gradient-to-br from-orange-50/30 via-white to-orange-50/20 p-8 shadow-inner"> | |
| <div className="relative flex mb-8 h-[700px] w-full flex-col items-center justify-center overflow-hidden rounded-2xl border border-gray-200/50 bg-gradient-to-br from-orange-50/30 via-white to-orange-50/20 p-8 shadow-inner"> |
| </div> | ||
| </div> | ||
| <blockquote className="text-sm leading-relaxed text-gray-700 flex-1">{content}</blockquote> | ||
| <div className="absolute top-3 right-3 opacity-8"> |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The opacity value opacity-8 is invalid. Tailwind CSS opacity classes use values from 0-100 in increments (e.g., opacity-10, opacity-20, etc.). This should be changed to a valid opacity value like opacity-10 or opacity-20.
amaan-bhati
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Gagan202005 Thanks for raising this pr and also for actively contributing to keploy. I reviewed this pr locally and i have a few suggestions:
- The section seems to have different background as compared to the landing background, the section needs to blend well with the background hence i would suggest to either match the same bg or make this section have a transparent background.
- The best practice here would be to make the section have no background at all since there might be a case we might be using the same section at different places in the site, hence it is better to make the section have no background at all so that it blends well with the same background.
- Here's a reference image for better understanding of what i am talking about:
- Importance of attention to detail here: There's only a minor difference in both the background but smhw causes discomfort to the eye causing a bad user experience, also it is would be good practices to not have a bg here so that the section blends everywhere we use it.
- There's only one row in this section where the testimonials are moving in a direction, the better ui would be having two marquees moving in opposite directions, I hope you get it, if you want any further clarifications, you can reach out to us on slack!
Signed-off-by: Gagan202005 <gagansinghal2005@gmail.com>
Signed-off-by: Gagan202005 <gagansinghal2005@gmail.com>
|
Hi @amaan-bhati Thanks a lot for the detailed review and suggestions — they were really helpful.
Testing : Screen.Recording.2026-01-09.at.2.54.49.AM.movCould you please take another look and let me know if everything looks good now? Thanks again! |
amaan-bhati
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Gagan202005 Thanks for following up on the previous review, i ran the changes on my localhost, i love the fact that when we hover over a tweet, there is a border effect which adds up to the ui in a good way. But there is still a clear difference between the background of this section and the background of the home page, this section still doesnt have a transparent background hence it is not blending with the background, although it does blend well at the bottom, something similar or complete transparence i the entire section would be great too.
Images for reference:
blends well at the bottom but there is still clear difference a the edges.
sir if we make whole section transparent, then there will be no blur effect at the edges |


Related Tickets & Documents
Fixes: keploy/keploy#3440
Description
Changes
Card Styling: Upgraded testimonial cards with modern design including:
Typography & Icons:
Layout Improvements:
max-w-7xl)px-5 sm:px-6)Heading & Subtitle:
Content Display:
Type of Change
Testing
Demo
testimonial.testing.mov
Checklist