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

Week 12: Design handoff - Alma and Etna #49

Open
wants to merge 37 commits into
base: main
Choose a base branch
from

Conversation

almaherris
Copy link

Netlify link

https://urban-spin.netlify.app/

Collaborators

[Caracal23]

Copy link

@AntonellaMorittu AntonellaMorittu left a comment

Choose a reason for hiding this comment

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

Hi Alma & Etna, congratulations for completing your week 12 project. It looks ✨ awesome ✨ The site looks very close to the design and followed our requirements. How did you like Tailwind? Seems like you tackled it in the right way and implemented a great result! How cool you created your own carousel! Now the design is not pixel perfect, but that's okay, here's a little things that can be taken into consideration to improve it even further:

  • Spaces and passings are not exactly the same
  • hovering state can be added to all elements and buttons
  • columns gap/proportions are slightly off compared to the ones in the design

I think you nailed it, so well done! ⭐

Choose a reason for hiding this comment

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

please format 🧹

Choose a reason for hiding this comment

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

here two, format to 2spaces indentation 👍

Copy link

@linda-f linda-f left a comment

Choose a reason for hiding this comment

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

You guys did a great job following the design.
We didn't do Tailwind therefore it is a bit hard to comment on that part but it looks really good and it seems to be working.
We really liked the carousel! 😵‍💫
//Izabel, Linda

@@ -0,0 +1,50 @@
import { ButtonFAQ } from "./reusable/ButtonFAQ";
Copy link

Choose a reason for hiding this comment

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

Nice that you made the button reusable!
//Izabel, Linda

touchEndX.current = e.touches[0].clientX
}

const handleTouchEnd = () => {
Copy link

Choose a reason for hiding this comment

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

Great job!!!!!!!!! 🥇
//Izabel, Linda

{feedback.feedback.map((_, index) => {
return (
<button
className="bg-grey-dot2 hover:bg-grey-dot1 w-2 h-2 rounded-full border-none outline-none mx-1 cursor-pointer"
Copy link

Choose a reason for hiding this comment

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

When sliding, the button doesn't get darker. We guess it should show the card the user is on at the moment.
//Izabel, Linda

import data from "../json/links.json"

export const Footer = () => {
const getLinks = (startIndex, endIndex) => {
Copy link

Choose a reason for hiding this comment

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

Nice touch that you made jsons for all the data.
//Izabel, Linda

return (
<div
className={`flex flex-col w-64 h-96 mb-28 mx-auto tablet:flex-row tablet:w-96 tablet:mx-8 tablet:items-center tablet:h-80 desktop:flex-col ${
currentCard === false && "hidden"
Copy link

Choose a reason for hiding this comment

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

Cool way to show the card that the user is currently on.
//Izabel, Linda

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.

None yet

4 participants