Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Create an HTML page with a catalog. Develop semantic page structure as shown on
- add `data-qa="card-hover"` (not just `hover`) to the link `Buy` inside the first card
- nav links color is not `black` anymore (nav links should have `#060b35` color)
- add the class `is-active` to the first link (`Apple`) in the navigation
- use `<main>` tag for cards container
- use `<main>` tag for cards container
- use the grid for cards with different numbers of columns:
- 1 for the smaller screens
- 2 starting at `488px`
Expand All @@ -33,8 +33,8 @@ This is possible because [we use the Parcel library](https://en.parceljs.org/scs
## Checklist

❗️ Replace `<your_account>` with your GitHub username and copy the links to the `Pull Request` description:
- [DEMO LINK](https://<your_account>.github.io/layout_catalog/)
- [TEST REPORT LINK](https://<your_account>.github.io/layout_catalog/report/html_report/)
- [DEMO LINK](https://kwz7.github.io/layout_catalog/)
- [TEST REPORT LINK](https://kwz7.github.io/layout_catalog/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
127 changes: 126 additions & 1 deletion src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,131 @@
</head>

<body>
<h1>Catalog</h1>
<header class="header">
<a
href="#"
class="header__logo-link"
>
<img
src="./images/logo.png"
alt="Moyo logo"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item "Repeated sizes and special colors are put to variables" — color: #34568b; is a hard-coded special color already defined as $color-title-hover in _variables.scss, so this should use that variable instead of the literal value.

class="header__logo-img"
/>
</a>

<nav class="nav">
<ul class="nav__list">
<li class="nav__item">
<a
href="#apple"
class="nav__link is-active"
>
Apple
</a>
</li>
<li class="nav__item">
<a
href="#samsung"
class="nav__link"
>
Samsung
</a>
</li>
<li class="nav__item">
<a
href="#smartphones"
class="nav__link"
>
Smartphones
</a>
</li>
<li class="nav__item">
<a
href="#laptops"
class="nav__link"
data-qa="nav-hover"
>
Laptops & Computers
</a>
</li>
<li class="nav__item">
<a
href="#gadgets"
class="nav__link"
>
Gadgets
</a>
</li>
<li class="nav__item">
<a
href="#tablets"
class="nav__link"
>
Tablets
</a>
</li>
<li class="nav__item">
<a
href="#photo"
class="nav__link"
>
Photo
</a>
Comment on lines 24 to +94

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #2: "Repeated sizes and special colors are put to variables" because header colors are still hard-coded (e.g., #fff for background, #060b35 for nav links, and #00acdc via --primary-blue) instead of using the SCSS variables from _variables.scss like $color-white, $color-text-accent, and $color-main. Consider @use 'variables' as *; in this file and replacing these literals with the corresponding variables to keep colors centralized.

</li>
<li class="nav__item">
<a
href="#video"
class="nav__link"
>
Video
</a>
</li>
</ul>
</nav>
</header>

<main class="cards-container">
<div
class="card"
data-qa="card"
>
<img
class="card__image"
src="images/imac.jpeg"
alt="iMac A1419"
/>

<h2 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h2>

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

<div class="card__review-container">
<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="card__review-count">Reviews: 5</div>
</div>

<div class="card__price-container">
<div class="card__price-label">Price:</div>
<div class="card__price">$2,199</div>
</div>

<a
class="card__buy"
href="#"
data-qa="card-hover"
>
Buy
</a>
</div>
</main>
</body>
</html>
93 changes: 93 additions & 0 deletions src/styles/_card.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
@use 'variables' as *;

.card {
box-sizing: border-box;
width: $card-width;
border: 1px solid $color-border;
border-radius: 5px;
padding: 32px 16px 16px;
background: $color-white;
transition: transform $transition-duration;

&:hover {
transform: scale(1.2);
}

&__image {
display: block;
margin: 0 auto 40px;
width: 160px;
height: 134px;
}

&__title {
margin: 0 0 4px;
font-size: 12px;
line-height: 18px;
font-weight: 500;
color: $color-text-accent;
transition: color $transition-duration;

.card:hover & {
color: $color-title-hover;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item "All components follow BEM and use SCSS" because class="logo-img" is not a BEM-style element or modifier of a block (it should be something like header__logo-img attached to the header block or a dedicated logo block).

}

&__code {
margin-bottom: 16px;
}

&__review-container {
display: flex;
justify-content: space-between;
align-items: flex-end;
margin-bottom: 24px;
}

&__review-count {
color: $color-text-accent;
}

&__price-container {
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: 16px;
}

&__price-label {
font-size: 12px;
line-height: 18px;
}

&__price {
font-size: 16px;
line-height: 18px;
font-weight: bold;
color: $color-text-accent;
}

&__buy {
display: flex;
justify-content: center;
align-items: center;
box-sizing: border-box;
height: 40px;
border: 1px solid $color-main;
border-radius: 5px;
font-size: 14px;
line-height: 16px;
font-weight: 700;
color: $color-white;
text-decoration: none;
text-transform: uppercase;
background-color: $color-main;
transition:
color $transition-duration, background-color $transition-duration;

&:hover {
color: $color-main;
background-color: $color-white;
}
}
}
29 changes: 29 additions & 0 deletions src/styles/_cards-container.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
@use 'variables' as *;

.cards-container {
display: grid;
grid-template-columns: $card-width;
justify-content: center;
gap: $gap-vertical $gap-horizontal;
padding: $container-padding-vertical $container-padding-horizontal;
max-width: calc(#{$card-width} * 4 + #{$gap-horizontal} * 3 + #{$container-padding-horizontal} * 2);
margin: 0 auto;
}

@media (min-width: 488px) {
.cards-container {
grid-template-columns: repeat(2, $card-width);
}
}

@media (min-width: 768px) {
.cards-container {
grid-template-columns: repeat(3, $card-width);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #2: "Repeated sizes and special colors are put to variables" because background-color: #fff; hard-codes the same white color you already defined as $color-white in _variables.scss; use the SCSS variable here instead of the literal.

@media (min-width: 1024px) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the checklist point "Repeated sizes and special colors are put to variables" because you hard-code #34568b here while $color-title-hover is defined in _variables.scss. Use the variable instead of the literal to keep special colors centralized.

.cards-container {
grid-template-columns: repeat(4, $card-width);
}
}
95 changes: 95 additions & 0 deletions src/styles/_header.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
:root {
--primary-blue: #00acdc;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Defining --primary-blue: #00acdc; duplicates the $color-main SCSS variable value instead of reusing it, which goes against the checklist item Repeated sizes and special colors are put to variables. Try relying on the existing SCSS variable (or deriving the CSS variable from it) so the color is not hard-coded twice.

}

body {
margin: 0;
}

ul {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item "Repeated sizes and special colors are put to variables" — card background #fff is a repeated special color and should be replaced with an SCSS variable (similar to $color-main, $color-text-accent, etc.).

margin: 0;
padding: 0;
list-style: none;
}

.header {
display: flex;
align-items: center;

padding: 0 50px;

height: 60px;
box-sizing: border-box;

background-color: #fff;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This still hard-codes #fff for the header background, even though $color-white is defined in _variables.scss. This violates checklist item: Repeated sizes and special colors are put to variables; consider using your SCSS color variable instead of the literal here.

justify-content: space-between;
box-shadow: 0 2px 4px rgba(0, 0, 0, 0.05);
}

.header__logo-link {
display: block;
height: 40px;
width: 40px;
Comment on lines +31 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item "Repeated sizes and special colors are put to variables" — you already have $color-title-hover defined, so the title hover color should use that variable instead of the hard-coded #34568b.

}
Comment on lines +30 to +33

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item "All components follow BEM and use SCSS" — the logo-img class in HTML is not aligned with your BEM naming (it should be something like header__logo or similar) and styled via SCSS as part of the Header block.


.header__logo-img {
display: block;
height: 40px;
width: 40px;
}

.nav__list {
display: flex;
padding: 0;
margin: 0;
align-items: center;
list-style: none;
}

.nav__link {
display: flex;
align-items: center;
height: 60px;

color: #060b35;

position: relative;
font-size: 12px;
font-weight: 500;
line-height: 100%;
transition: color 0.3s;
white-space: nowrap;
text-decoration: none;
text-transform: uppercase;
font-family: Roboto, sans-serif;
}

.nav {
display: flex;
}

.nav__item {
margin-left: 20px;
}

.nav__item:first-child {
margin-left: 0;
}

.nav__link.is-active {
color: var(--primary-blue);
}

.nav__link.is-active::after {
content: '';
position: absolute;
bottom: 0;
width: 100%;
height: 4px;
border-radius: 2px;
background-color: var(--primary-blue);
}

.nav__link:hover {
color: var(--primary-blue);
}
22 changes: 22 additions & 0 deletions src/styles/_stars.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
.stars {
display: flex;

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item "Repeated sizes and special colors are put to variables" — card background #fff should be represented by a SCSS variable (similar to other colors) rather than a hard-coded literal.

background-position: center;

&:last-child {
Comment on lines +4 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item "Repeated sizes and special colors are put to variables" — sizes like 200px, 48px, and 40px are repeated across the layout and should be extracted into SCSS variables and used here instead of hard-coded literals.

margin-right: 0;
}
}

@for $i from 1 through 5 {
&--#{$i} &__star:nth-child(-n + #{$i}) {
background-image: url('../images/star-active.svg');
}
}
}
Loading
Loading