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.

Sign up for GitHub

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

Jump to bottom

add task solution #4631

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

add task solution #4631

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb0727f
add task solution
labelsavchuk Aug 3, 2024
c834bf4
spaces between chuld elements, btn-hover-color variable, nesting for …
labelsavchuk Oct 13, 2024
c0515d5
variables for colors not for background, fix alt for img, font declar…
labelsavchuk Oct 14, 2024
5da603e
split index.scss on elements, informative alt for img, from .item to …
labelsavchuk Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Product cards

Create a pages with product card using `flexbox`, `BEM` and `SCSS` based on [this mockup](https://www.figma.com/file/ojkArVazq7vsX0nbpn9CxZ/Moyo-%2F-Catalog-(ENG)?node-id=11325%3A2287&mode=dev).
Create a pages with product card using `flexbox`, `BEM` and `SCSS` based on [this mockup](<https://www.figma.com/file/ojkArVazq7vsX0nbpn9CxZ/Moyo-%2F-Catalog-(ENG)?node-id=11325%3A2287&mode=dev>).

> Here are the [Layout Tasks Instructions](https://mate-academy.github.io/layout_task-guideline)

Expand All @@ -20,8 +20,8 @@ Create a pages with product card using `flexbox`, `BEM` and `SCSS` based on [thi

❗️ Replace `<your_account>` with your Github username and copy the links to `Pull Request` description:

- [DEMO LINK](https://<your_account>.github.io/layout_product-cards/)
- [TEST REPORT LINK](https://<your_account>.github.io/layout_product-cards/report/html_report/)
- [DEMO LINK](https://labelsavchuk.github.io/layout_product-cards/)
- [TEST REPORT LINK](https://labelsavchuk.github.io/layout_product-cards/report/html_report/)

❗️ Copy this `Checklist` to the `Pull Request` description after links, and put `- [x]` before each point after you checked it.

Expand Down
55 changes: 54 additions & 1 deletion src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,61 @@
rel="stylesheet"
href="./styles/index.scss"
/>
<link
rel="preconnect"
href="https://fonts.googleapis.com"
/>
<link
rel="preconnect"
href="https://fonts.gstatic.com"
/>
<link
href="https://fonts.googleapis.com/css2?family=Poppins:ital,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,500;1,600;1,700;1,800;1,900&family=Roboto:ital,wght@0,100;0,300;0,400;0,500;0,700;0,900;1,100;1,300;1,400;1,500;1,700;1,900&display=swap"
rel="stylesheet"
/>
Copy link

@VolodymyrKirichenko VolodymyrKirichenko Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

use only the fonts u need. u only use a couple of fonts, but u import them all

</head>
<body>
<h1>Product cards</h1>
<div
class="card"
data-qa="card"
>
<img
src="./images/imac.jpeg"
alt="phtot of i-mac"
class="card__photo"
Copy link

Choose a reason for hiding this comment

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

add more informative alt

Copy link

Choose a reason for hiding this comment

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

not fixed.

Copy link

Choose a reason for hiding this comment

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

add more informative alt

Not fixed
image

/>

<div class="item">
<div class="item__name">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</div>

<div class="item__code">Product code: 195434</div>

<div class="review">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
</div>
<div class="review__count">Reviews: 5</div>
</div>

<div class="item__price">
<div class="item__price-header">Price:</div>
<div class="item__price-value">$2,199</div>
Copy link

Choose a reason for hiding this comment

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

Use semantic tags where possible instead of only divs

</div>
</div>

<a
href="#1"
class="card__btn"
data-qa="hover"
>
Buy
</a>
</div>
</body>
</html>
126 changes: 126 additions & 0 deletions src/styles/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,129 @@
$bc-image: url(/src/images/star-active.svg);
Copy link

Choose a reason for hiding this comment

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

do u need this variable?But why?u used it only once

$btn-color: #00acdc;
$main-acent: #060b35;
$secondary: #616070;
$btn-hover-color: #fff;

body {
margin: 0;
font-family: Roboto, sans-serif;
font-weight: 400;
font-size: 10px;
line-height: 14px;
}

.card {
Copy link

Choose a reason for hiding this comment

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

Create a separate file per each styles block
image

font-family: Roboto, sans-serif;
font-weight: 400;
font-size: 10px;
line-height: 14px;
Copy link

Choose a reason for hiding this comment

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

do u need it?
I see that u set the fonts globally on the body tag

Suggested change
font-family: Roboto, sans-serif;
font-weight: 400;
font-size: 10px;
line-height: 14px;


box-sizing: border-box;
width: 200px;
border: 1px solid #f3f3f3;
Copy link

Choose a reason for hiding this comment

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

use variables for all colors

border-radius: 5px;
padding: 32px 16px 16px;

&__photo {
display: block;
margin: 0 auto 40px;
width: 160px;
height: 134px;
}
&__btn {
display: flex;
align-items: center;
justify-content: center;
color: #fff;
text-transform: uppercase;
text-decoration: none;
font-size: 14px;
font-weight: 700;
line-height: 16px;

width: 100%;
height: 40px;
box-sizing: border-box;
border: 1px solid $btn-color;
border-radius: 5px;
background: $btn-color;

&:hover {
background: $btn-hover-color;
color: $btn-color;
}
}
}

.item {
Copy link

Choose a reason for hiding this comment

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

What item is it?
Names should be descriptive

&__name {
font-size: 12px;
font-weight: 500;
line-height: 18px;
text-align: left;
color: $main-acent;
margin-bottom: 4px;
}

&__code {
line-height: 14px;
text-align: left;
margin-bottom: 16px;
color: $secondary;
}

&__price {
display: flex;
justify-content: space-between;
line-height: 18px;
margin-bottom: 16px;
color: $secondary;

&-value {
font-size: 16px;
font-weight: 700;
text-align: right;
color: #060b35;
}

&-header {
font-size: 12px;
text-align: left;
color: #616070;
}
}
}

.stars {
display: flex;
flex-direction: row;

&__star {
background-image: url(/src/images/star.svg);
background-position: center;
height: 16px;
margin-right: 4px;
width: 16px;
background-repeat: no-repeat;
Copy link

Choose a reason for hiding this comment

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

Suggested change
background-image: url(/src/images/star.svg);
background-position: center;
height: 16px;
margin-right: 4px;
width: 16px;
background-repeat: no-repeat;
background: ............;
height: 16px;
margin-right: 4px;
width: 16px;

u can use just background for it

}

&.stars--4 :nth-child(-n + 4) {
Copy link

Choose a reason for hiding this comment

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

It should be the same functionality as in the stars task, if modifier is --3 you should show 3 active stars as well

background-image: $bc-image;
}

& :last-of-type {
margin: 0;
}
}

.review {
Copy link

Choose a reason for hiding this comment

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

there 2 BEM blocks card and stars each in it own file
image

display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: 24px;

&__count {
text-align: right;
color: $main-acent;
}
}
Loading