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

solution #214

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

solution #214

wants to merge 28 commits into from

Conversation

IzzetYusufov
Copy link

@IzzetYusufov IzzetYusufov commented Aug 31, 2023

DEMO LINK

Проверьте пожалуйста повторно, необходимо для эмплоймента! Спасибо!

Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

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

image

Copy link

@anastasiiavorobiova anastasiiavorobiova left a comment

Choose a reason for hiding this comment

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

Good work! Will be waiting for the next pages

@IzzetYusufov
Copy link
Author

there is no more pages, API doesn't have any other data, such as tablets or accessories

Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

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

Let's to improve your page

  1. Add transition for hover effects here
image
  1. Add a favicon on the page
image
  1. Add more hover effects for this button
image

@IzzetYusufov
Copy link
Author

Прошу проверить, вроде все сделал, вопрос остается на счет страниц которых нет и ошибок, стоит ли потом заморочится более красивее сделать, с какой то картинкой и стоит ли заморочится заполнить как то потом райтс и контактс, если контактс еще можно придумать то вот райтс не знаю что писать

Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

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

  1. I think that when you click the button below again, it would be logical to remove the product from the cart and change the text to add it again
image
  1. Regarding the pages that are not there, I think it is worth bothering and implementing the design a little better, for example, that the pages are in the process of development and some kind of picture, or something similar

Copy link

@IvanFesenko IvanFesenko left a comment

Choose a reason for hiding this comment

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

Where is bottom card border
2023-09-15_15h50_04
2023-09-15_15h54_36

No need to show this if there are no results message
2023-09-15_15h51_27

0 value should not be in card
2023-09-15_15h52_26

Pay attention on placeholder

2023-09-15_15h53_48.mp4

@IzzetYusufov
Copy link
Author

на гитхабе и локально на компьютере нет проблем с изображением карточек, как найти проблему?

Copy link

@IvanFesenko IvanFesenko left a comment

Choose a reason for hiding this comment

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

Forgot to fix bottom border

2023-09-15_16h30_54.mp4

@IzzetYusufov
Copy link
Author

вроде пофиксил на файрфокс была проблема вроде как

@IzzetYusufov
Copy link
Author

у меня во всех браузерах с бордерами все ок сейчас, проверил на 4 браузерах

Copy link

@IvanFesenko IvanFesenko left a comment

Choose a reason for hiding this comment

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

well done

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.

4 participants