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: add testimonial carousel #1704

Closed

Conversation

ashutosh-rath02
Copy link
Contributor

Description

  • Added a carousel in "What the Experts are saying section" to improve the visual appearance and enhance the focus of the user to a single testimonial at a time.
  • Added pagination feature which allow user to access the required testimonial.
  • Made changes to components/Testimonials.js and pages/index.js

Related issue(s)

Fixes #1696

After

caro.mp4

@netlify
Copy link

netlify bot commented May 23, 2023

Deploy Preview for asyncapi-website failed.

Built without sensitive environment variables

Name Link
🔨 Latest commit 453ee50
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/65e87bf093ecab00087598d0

@github-actions
Copy link

github-actions bot commented May 23, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 54
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🔴 PWA 30

Lighthouse ran on https://deploy-preview-1704--asyncapi-website.netlify.app/

@ashutosh-rath02
Copy link
Contributor Author

ashutosh-rath02 commented May 23, 2023

@akshatnema I guess the previous PR(#1698 ) had some issues, so I had to make new PR and this one has passed all checks.
Issue- #1696

@Mayaleeeee
Copy link
Member

Mayaleeeee commented May 24, 2023

Hi @Lucif3r-in
You have done an excellent job, and I appreciate your efforts. However, I have a few suggestions to enhance the overall experience further.

  • Please create sufficient spacing between the header and the cards. This will help improve the visual appeal and readability of the content.
  • Use pagination icons beside the cards on the left and right sides to provide users with a more intuitive navigation experience.
  • To ensure that users can focus on one card at a time, I recommend making the active card more significant than the others. Alternatively, you could add a subtle shadow to distinguish the active card from the rest.

@ashutosh-rath02
Copy link
Contributor Author

Hi @Lucif3r-in
You have done an excellent job, and I appreciate your efforts. However, I have a few suggestions to enhance the overall experience further.

  • Please create sufficient spacing between the header and the cards. This will help improve the visual appeal and readability of the content.
  • Use pagination icons beside the cards on the left and right sides to provide users with a more intuitive navigation experience.
  • To ensure that users can focus on one card at a time, I recommend making the active card more significant than the others. Alternatively, you could add a subtle shadow to distinguish the active card from the rest.

I will add the arrows for the second point and decrease the opacity of the inactive cards to make it distinguishable.

@Mayaleeeee
Copy link
Member

Hi @Lucif3r-in
You have done an excellent job, and I appreciate your efforts. However, I have a few suggestions to enhance the overall experience further.

  • Please create sufficient spacing between the header and the cards. This will help improve the visual appeal and readability of the content.
  • Use pagination icons beside the cards on the left and right sides to provide users with a more intuitive navigation experience.
  • To ensure that users can focus on one card at a time, I recommend making the active card more significant than the others. Alternatively, you could add a subtle shadow to distinguish the active card from the rest.

I will add the arrows for the second point and decrease the opacity of the inactive cards to make it distinguishable.

Alright, let's give it a try and see how it works.
Weldone!

@ashutosh-rath02
Copy link
Contributor Author

@Mayaleeeee Check pls I have done the asked changes. You can check it here https://deploy-preview-1704--asyncapi-website.netlify.app/

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

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

@Lucif3r-in Thanks for improving the testimonials section. I have couple of changes from my side:

image

The navigation arrow buttons are not easily visible in the section. Kindly increase of opacity of the buttons.
Secondly. the carousel should have the property of auto-scroll so that it should show other testimonials automatically to the users.

components/Testimonial.js Outdated Show resolved Hide resolved
components/Testimonial.js Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
@ashutosh-rath02
Copy link
Contributor Author

@Lucif3r-in Thanks for improving the testimonials section. I have couple of changes from my side:

image

The navigation arrow buttons are not easily visible in the section. Kindly increase of opacity of the buttons. Secondly. the carousel should have the property of auto-scroll so that it should show other testimonials automatically to the users.

Can you tell me at what screen size are you facing the issue as I am unable to reproduce the issue? Still I will increase the opacity.

@ashutosh-rath02
Copy link
Contributor Author

@akshatnema Check the changes please. I had to add react-icons for the arrows we have used. You can suggest me any suitable changes to replace those.

@Mayaleeeee
Copy link
Member

@Mayaleeeee Check pls I have done the asked changes. You can check it here https://deploy-preview-1704--asyncapi-website.netlify.app/

Hello @Lucif3r-in thanks for working on this, and I apologize for the delayed response. After reviewing the website on my laptop, I find the overall design pleasing. However, the mobile view seems to require some adjustments to ensure a better user experience. Currently, it appears that you have attempted to maintain the same layout as the desktop version by compressing the content within the card. Unfortunately, this approach has resulted in a slim and narrow card view, resembling a rectangle shape that feels awkward.

To adjust this and maintain a consistent square size across both desktop and mobile, you can simply reduce the size of the square shape and its corresponding content. For instance, if you use a 24px header on the desktop, consider reducing it to 16px for mobile.

I have attached a screenshot of the mobile view below to provide a clearer understanding.
Screenshot_20230528-183608

@ashutosh-rath02
Copy link
Contributor Author

I tried doing the same @Mayaleeeee but the text overflows at smaller screen. What do you suggest should I make the text scrolable?

@Mayaleeeee
Copy link
Member

I tried doing the same @Mayaleeeee but the text overflows at smaller screen. What do you suggest should I make the text scrolable?

Oops
Okay, can you send me the link to the figma file?

@ashutosh-rath02
Copy link
Contributor Author

ashutosh-rath02 commented May 29, 2023

I tried doing the same @Mayaleeeee but the text overflows at smaller screen. What do you suggest should I make the text scrolable?

Oops Okay, can you send me the link to the figma file?

I will make some changes and send you a video. I don't have a figma file actually, I just coded it. @Mayaleeeee What do you say should I remove these animations of 3 cards and place a single card instead?

@ashutosh-rath02
Copy link
Contributor Author

@akshatnema @Mayaleeeee I am adding a new video. If you approve we can use this carousel instead to avoid readability issue in the previous carousel.

carousel.mp4

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

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

image

Text inside the component is very small as compared to normal text inside website in mobile view. Kindly increase the font-size in mobile view. Also, you can think of using TextTruncate component for the description in mobile view and then having a Read More option to view full content.

Secondly. the carousel should have the property of auto-scroll so that it should show other testimonials automatically to the users.

Kindly work on this feature too if possible.

config/content/Testimonial.js Outdated Show resolved Hide resolved
components/Testimonial.js Outdated Show resolved Hide resolved
@ashutosh-rath02
Copy link
Contributor Author

image

Text inside the component is very small as compared to normal text inside website in mobile view. Kindly increase the font-size in mobile view. Also, you can think of using TextTruncate component for the description in mobile view and then having a Read More option to view full content.

Secondly. the carousel should have the property of auto-scroll so that it should show other testimonials automatically to the users.

Kindly work on this feature too if possible.

So should the card size increase on clicking Read More? As increasing text size it overflows the card.

@derberg derberg requested a review from akshatnema June 22, 2023 12:21
@derberg
Copy link
Member

derberg commented Jun 22, 2023

@akshatnema I think there is an open question for you, but also commits that you can review

@akshatnema
Copy link
Member

@Lucif3r-in Can you please resolve the conflicts in this PR?

package.json Outdated
@@ -79,6 +79,7 @@
"react-dom": "^17.0.2",
"react-ga": "^3.1.2",
"react-gtm-module": "^2.0.11",
"react-icons": "^4.8.0",
Copy link
Member

Choose a reason for hiding this comment

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

Are we using these icons anywhere in the PR? I don't see any imports for this library.

components/Testimonial.js Outdated Show resolved Hide resolved
components/Testimonial.js Outdated Show resolved Hide resolved
components/Testimonial.js Show resolved Hide resolved
<ArrowLeft className='w-4'/>
</div>
<div className="flex flex-col justify-center items-center">
<div className="relative h-[300px] w-[300px] md:w-[600px] md:h-[450px] overflow-hidden">
Copy link
Member

Choose a reason for hiding this comment

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

Don't specify height for the box, as you may not know how long can be testimonial text. Moreover, using overflow-hidden is not a good practice. You can allow Read More option inside box to expand the testimonial box.

Copy link
Contributor Author

@ashutosh-rath02 ashutosh-rath02 Oct 1, 2023

Choose a reason for hiding this comment

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

For some reason I cant set the height to auto... When I am doing height-auto or max-content the cards just disappears. The Read more part is causing some issue also, causing this PR to delay

Copy link
Member

@Mayaleeeee Mayaleeeee Oct 2, 2023

Choose a reason for hiding this comment

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

Weldone @akshatnema

Hello @Lucif3r-in, thank you.
Where can I review the updated version of the testimonial section? Or is it still the same link in the deploy preview column?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same link @Mayaleeeee

Copy link
Member

@Mayaleeeee Mayaleeeee Oct 2, 2023

Choose a reason for hiding this comment

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

  1. The icon here is not centered in the middle.
    Screenshot 2023-10-02 134612

  2. You need to change the card background color to white and add elevation to match other cards on the website.
    Screenshot 2023-10-02 173125

Thank you @Lucif3r-in

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

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

Kindly add these files inside a new folder Testimonials.

@akshatnema
Copy link
Member

@Lucif3r-in any updates?

@ashutosh-rath02
Copy link
Contributor Author

@Lucif3r-in any updates?

I am having problem with the Read more button else everything seems good

@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Dec 8, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 31
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

Lighthouse ran on https://deploy-preview-1704--asyncapi-website.netlify.app/

@sambhavgupta0705
Copy link
Member

@ashutosh-rath02 any update?

@sambhavgupta0705
Copy link
Member

sambhavgupta0705 commented Mar 7, 2024

@ashutosh-rath02 the build is breaking please try running npm run build on your local machine to know about the errors

@sambhavgupta0705
Copy link
Member

@ashutosh-rath02 any updates??

@sambhavgupta0705
Copy link
Member

closing this PR as there is no update on this from the contributor from last 1 month
It can be reopened again if he comes back

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.

Enhancement Request: Improving the What the experts are saying Section on the AsyncAPI Website
6 participants