Amon• 2,560
@A-amon
Posted
Hello! Awesome work (And clean code too~ 😉)
I have a few suggestions:
- To fix the box-shadow, you can set the box-shadow on your .btn-container instead of your .btn. Then, set max-width:fit-content; and border-radius:50px;
- Your slider HTML looks good. I am not that well-versed on slider's accessibility but I guess it's kind of similar to tabs (?) 🤔
- So I would have only one set of prev and next buttons (instead of one for each slide). And when each button is clicked, the focus is moved to the new slide.
- Btw, neat JS code. Maybe add some comments too~ 😀
Marked as helpful
0
Yuko Horita• 645
@Sloth247
Posted
@A-amon Thank you for your feedback, I will fix the box-shadow. I have a few questions for you;
- the accessibility for slider, are you asking me a question or is it a suggestion? Do you mean tab-index as "tabs"? Sorry I didn't get your meaning.
- If you have only one buttons, how can you change the image and texts for each person?
Your comments are helpful.
0
Amon• 2,560
@A-amon
Posted
@Sloth247 Oops! Sorry~ 😂 I was suggesting lmao. From what I can imagine/think of right now, there would be 1 div for each person-related content. Clicking the button will toggle the display of the new slide (previously hidden - could be display:none; or hidden attribute) and focus on it.
Marked as helpful
0
