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: 💄 New carousel styles are adapted #1287

Conversation

miusarname2
Copy link

New style of the carousel is adapted, where a ‘shading’ is created on the sample image, and a play icon is shown over them, with a user-friendly animation.

Resolves #1280

What changes did you make and why did you make them?

Modified the ShortsCards component, added an IconButton, and modified the hover of the ShortsCards.

Did you run tests? Share screenshot of results:

yep

image

How did you find us? (GitHub, Google search, social media, etc.):

Well, I was looking for open source projects on the internet that were good for society and I came across them by chance (and because I was also a "victim").

New style of the carousel is adapted, where a ‘shading’ is created on the sample image, and a play icon is shown over them, with a user-friendly animation.
Copy link

vercel bot commented Feb 3, 2025

@miusarname2 is attempting to deploy a commit to the Chayn Team on Vercel.

A member of the Team first needs to authorize it.

@kyleecodes kyleecodes self-requested a review February 3, 2025 20:51
@kyleecodes kyleecodes self-assigned this Feb 3, 2025
Copy link
Member

@kyleecodes kyleecodes left a comment

Choose a reason for hiding this comment

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

LGTM! The shading is great. @miusarname2 Were you able to implement the fade-out on the right side (as noted in the issue description)? We're happy to merge this PR as-is, but may keep the issue open for the fade-out.

@miusarname2
Copy link
Author

I had forgotten that little detail, I'll get to work on it tomorrow.

Adjust the carousel styles, adjusting them to the desired standards, and as close to the design as possible, introducing a mapping, so that the cards that are larger than the given number are shown at 0.5, and the correct ones at opacity 1.
A console.log, which was not supposed to be where it was, is deleted.
@miusarname2 miusarname2 force-pushed the miusarname2/Upgrade-Carousel-Designs branch from b94b02e to a4aee84 Compare February 8, 2025 13:11
@kyleecodes
Copy link
Member

@miusarname2 thank you for your swift PR. Unfortunately due to recent upgrades made on this app while you worked on this PR (this App router nextjs migration), this PR was flagged for merge conflicts in which we cannot resolve. This is due to changes in file structure.

We can still get this merged, your code should easily transfer to new files with only slight modifications. But first, you will need to close this PR, update your forked develop branch, create a new feature branch, and apply these changes to that new branch as follows:

  1. Changes in components/cards/ShortsCard.tsx can be re-applied in the same file (this path exists as-is in the new file structure).
  2. Changes in components/common/Carousel.tsx can also be re-applied in the same file.
  3. Changes in pages/courses/index.tsx will need to be moved to components/pages/CoursesPage.tsx as pages/courses/index.tsx no longer exists.

Then we can merge your PR ASAP. Thanks for your patience and let us know if you have any questions.

@miusarname2
Copy link
Author

Ok, I will make the changes and reopen the PR.

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.

Upgrade Carousel Designs
2 participants