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

Upgrade Carousel Designs #1280

Open
4 tasks
eleanorreem opened this issue Jan 24, 2025 · 13 comments · May be fixed by #1287
Open
4 tasks

Upgrade Carousel Designs #1280

eleanorreem opened this issue Jan 24, 2025 · 13 comments · May be fixed by #1287
Assignees
Labels
bug-fix Fix a bug complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days help wanted Extra attention is needed priority: high Should be prioritized immediately, likely blocking other tasks. state: approved Ready to go. Not blocked or pending. UX User experience design web-development

Comments

@eleanorreem
Copy link
Contributor

Overview

We had some designs for our Carousel component which we didn't fulfill due to time constraints. Below is the design.

Image Image

The improvements I want to try are:

  • faded out slide on the right
  • hover state with the play icon.

I had a brief look at I have a feeling this might not be possible with the nuka carousel implementation so if at the end of it, you can't find a way to do it, that's okay!

Action Items

  • Take a look at nuka-carousel
  • Take a look at our implementation. Any feedback on improving the implementation is welcome.
  • See if you can make the carousel closer to the designs.
  • Ensure the carousel works on the home page and also the /courses page.
@eleanorreem eleanorreem added bug-fix Fix a bug complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days help wanted Extra attention is needed priority: high Should be prioritized immediately, likely blocking other tasks. state: approved Ready to go. Not blocked or pending. labels Jan 24, 2025
@miusarname2
Copy link

miusarname2 commented Jan 31, 2025

Could I try to solve it myself, I have just read the documentation.

@salmafadlabdulrahman
Copy link

I'm going to try to fix the issue. and get closer to the design as possible.

@kyleecodes kyleecodes added web-development UX User experience design labels Jan 31, 2025
@kyleecodes
Copy link
Member

Hi @miusarname2 and @salmafadlabdulrahman thanks for your interest! Please note: this is an "advanced complexity level" issue, which may not have a solution. If there is no solution, a summary of the troubleshooting steps you've taken (and any ideas you have) will work to mark this issue as resolved.

Because you both commented within a short time (and this is an issue that could benefit from additional developers insight) would you both be open to working on this issue collaboratively? Similarly, you can both be assigned and open separate PRs. Otherwise, we have to assign based on who commented first.

@salmafadlabdulrahman
Copy link

After reviewing the issue and exploring the codebase, I realize that this problem requires a deeper understanding of the project structure. As I'm still familiarizing myself with the codebase, I believe it’s best for @miusarname2 to continue working on this issue.

That being said, I’m genuinely interested in contributing to the project and would love to assist with any beginner-friendly or intermediate tasks where I can add value. Thank you for the opportunity, and I look forward to contributing more in the future!

@miusarname2
Copy link

I was looking at the possibility of overwriting the styles a bit aggressively, which is the only viable way I see, that is to say forcibly overwrite at the end of ‘!important’, those styles, I do not know what you think of my idea, is what I've been exploring.

@miusarname2
Copy link

for example, this is what I've been doing, sorry, I was resting at the weekend.

avance.mp4

@miusarname2
Copy link

miusarname2 commented Feb 3, 2025

ok, and I improve a little, to something as desired, just need to tell me if you like it (obviously I will apply the colours), or remove the colour of Hover is to say that change colour

captura-_4_.mp4

the only thing that worries me is if it complies with the standards used, as I'm not very sure about using them, here is a sample of the changed code

<Box
              className="play-button"
              sx={{
                position: 'absolute',
                top: '50%',
                left: '50%',
                transform: 'translate(-50%, -50%) scale(0.8)',
                opacity: 0,
                transition: 'opacity 0.3s ease, transform 0.3s ease',
              }}
            >
              <IconButton sx={{ color: 'white', backgroundColor: 'rgba(0, 0, 0, 0.5)' }}>
                <PlayArrowIcon sx={{ fontSize: 40 }} />
              </IconButton>
            </Box>
          </Box>

I'm going to finish it, and get closer to the design with the colours, I'll commit, and if you want you can clone my repo, and look at it.

Copy link
Contributor

github-actions bot commented Feb 3, 2025

Thank you @miusarname2 you have been assigned this issue!
Please follow the directions in our Contributing Guide. We look forward to reviewing your pull request. ✨


Support Chayn's mission? ⭐ Please star this repo to help us find more contributors like you!
Learn more about Chayn's impact here. 🌸

@miusarname2
Copy link

I already made the commit in my personal repository, and this was the result, you can tell me if I start the pull request, what I'm still unsure is that it meets the code standards, in that sense I will see if it can be improved.

captura-_5_.mp4

The repository with the modifications

@miusarname2
Copy link

Today from 2pm to 3pm GMT-5, I will re-read the documentation and do the PR.

@miusarname2 miusarname2 linked a pull request Feb 3, 2025 that will close this issue
@miusarname2
Copy link

I was looking at how to do it, and I know that it can be done, and apply to that specific case, but I have a technical doubt and I am considering the possibility of modifying the properties of the ShortCards, and add a new one that is type ‘UsedShorts’, or something like that, but I'm a little scared, because it would create a property, which is only going to be used once, is it worth it or I explore other options? @kyleecodes

@miusarname2
Copy link

forget everything, I found a way to do it, but I think I will have to modify another module.

@miusarname2
Copy link

miusarname2 commented Feb 4, 2025

At the moment I'm have this, I think I'm going to have to look at how to take the swipe event.

captura-_6_.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix Fix a bug complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days help wanted Extra attention is needed priority: high Should be prioritized immediately, likely blocking other tasks. state: approved Ready to go. Not blocked or pending. UX User experience design web-development
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants