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

Develop #4819

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

Develop #4819

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
033c050
changes in files
Artemida1609 Oct 6, 2024
577ebb7
changes in files
Artemida1609 Oct 6, 2024
93f4f51
changes in files
Artemida1609 Oct 6, 2024
a7d6210
changes in files
Artemida1609 Oct 6, 2024
e16475c
changes in files
Artemida1609 Oct 6, 2024
5a9986d
changes in files
Artemida1609 Oct 6, 2024
0e6027b
changes in files
Artemida1609 Oct 6, 2024
76d4853
changes in files
Artemida1609 Oct 6, 2024
c9b1ca3
changes
Artemida1609 Oct 6, 2024
8e4ee83
changes
Artemida1609 Oct 6, 2024
4e0cd2b
some changes
Artemida1609 Oct 6, 2024
ddea04e
changes
Artemida1609 Oct 6, 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
82 changes: 41 additions & 41 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,41 @@
{
"name": "product-cards",
"version": "1.0.0",
"description": "Frontend practice with product cards",
"scripts": {
"init": "mate-scripts init",
"start": "mate-scripts start",
"format": "prettier --ignore-path .prettierignore --write './src/**/*.{html,css,scss}'",
"lint": "npm run format && mate-scripts lint",
"test:only": "mate-scripts test",
"build": "mate-scripts build",
"deploy": "mate-scripts deploy",
"update": "mate-scripts update",
"postinstall": "npm run update",
"test": "npm run lint && npm run test:only"
},
"keywords": [],
"author": "Mate Academy",
"license": "GPL-3.0",
"devDependencies": {
"@linthtml/linthtml": "^0.9.6",
"@mate-academy/backstop-config": "latest",
"@mate-academy/bemlint": "latest",
"@mate-academy/linthtml-config": "latest",
"@mate-academy/scripts": "^1.8.6",
"@mate-academy/stylelint-config": "latest",
"@parcel/transformer-sass": "^2.12.0",
"backstopjs": "6.3.23",
"jest": "^29.7.0",
"parcel": "^2.12.0",
"prettier": "^3.3.2",
"stylelint": "^16.7.0",
"stylelint-scss": "^6.4.1"
},
"mateAcademy": {
"projectType": "layout",
"linters": {
"javascript": false
}
}
}
{
"name": "product-cards",
"version": "1.0.0",
"description": "Frontend practice with product cards",
"scripts": {
"init": "mate-scripts init",
"start": "parcel ./src/index.html --open",
"format": "prettier --ignore-path .prettierignore --write './src/**/*.{html,css,scss}'",
"lint": "npm run format && mate-scripts lint",
"test:only": "mate-scripts test",
"build": "mate-scripts build",
"deploy": "mate-scripts deploy",
"update": "mate-scripts update",
"postinstall": "npm run update",
"test": "npm run lint && npm run test:only"
},
"keywords": [],
"author": "Mate Academy",
"license": "GPL-3.0",
"devDependencies": {
"@linthtml/linthtml": "^0.9.6",
"@mate-academy/backstop-config": "latest",
"@mate-academy/bemlint": "latest",
"@mate-academy/linthtml-config": "latest",
"@mate-academy/scripts": "^1.8.6",
"@mate-academy/stylelint-config": "latest",
"@parcel/transformer-sass": "^2.12.0",
"backstopjs": "6.3.23",
"jest": "^29.7.0",
"parcel": "^2.12.0",
"prettier": "^3.3.2",
"stylelint": "^16.7.0",
"stylelint-scss": "^6.4.1"
},
"mateAcademy": {
"projectType": "layout",
"linters": {
"javascript": false
}
}
}
4 changes: 2 additions & 2 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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://Artemida1609.github.io/layout_product-cards/)
- [TEST REPORT LINK](https://Artemida1609.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
Binary file added src/images/imac.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 3 additions & 3 deletions src/images/star.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
61 changes: 60 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,67 @@
rel="stylesheet"
href="./styles/index.scss"
Copy link

Choose a reason for hiding this comment

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

According to Code Style Rules ❗️, CSS files should be linked with a .css extension, not .scss. Please change the file extension in the href attribute.

Copy link

Choose a reason for hiding this comment

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

According to the Code Style Rules ❗️, stylesheets with a .scss extension should be included as regular .css files in HTML. You should compile .scss to .css and then link to the .css file in your HTML.

Copy link

Choose a reason for hiding this comment

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

According to the Code Style Rules ❗️, SCSS files should be linked as CSS files in HTML. You need to compile your SCSS to CSS and link the compiled CSS file here instead.

/>

<link
rel="preconnect"
href="https://fonts.googleapis.com"
/>
<link
rel="preconnect"
href="https://fonts.gstatic.com"
crossorigin="anonymous"
/>
<link
href="https://fonts.googleapis.com/css2?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"
/>
</head>
<body>
<h1>Product cards</h1>
<div
class="card"
data-qa="card"
>
<img
src="./images/imac.jpeg"
Copy link

Choose a reason for hiding this comment

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

The image file extension in the src attribute should match the actual file extension. The file is named imac.png but here it is referenced as imac.jpeg. Please correct the file extension.

Copy link

Choose a reason for hiding this comment

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

The image source path is incorrect, it should be ./images/imac.png as per the file structure provided. Make sure the file extension and the path are correct.

alt="imac image"
class="card__image"
/>

<h3 class="card__title">
APPLE A1419 iMac 27" Retina
<br />
5K Monoblock (MNED2UA/A)
Copy link

Choose a reason for hiding this comment

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

Avoid using <br /> to create line breaks in text content. It's better to control the layout using CSS. If these are meant to be separate lines, consider wrapping them in separate elements.

</h3>

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

<div class="card__rating">
<div class="stars">
Copy link

Choose a reason for hiding this comment

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

Each star should have a class stars__star instead of stars-star following the BEM methodology as per the Code Style Rules ❗️.

<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>

<h3 class="card__reviews">Reviews: 5</h3>
Copy link

Choose a reason for hiding this comment

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

The element with class card__reviews should be a paragraph <p> instead of a heading <h3>, because it's not a heading but a piece of information related to the product. Reserve heading tags for titles and headings only.

</div>

<div class="card__price-block">
<h3 class="card__price-title">Price:</h3>

<h1 class="card__price-amount">$2,199</h1>
Copy link

Choose a reason for hiding this comment

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

Heading tags should be used to introduce content. Using an <h1> tag for the price amount is not semantically correct as it does not represent a heading of a section or the document. Consider using a different tag, like <p> or <span>, with appropriate class names for styling.

</div>

<button class="card__buy-button">
Copy link

Choose a reason for hiding this comment

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

The button element should not contain the anchor <a> element. Instead, apply the card__buy-button class directly to the anchor tag and remove the button element. This is because semantically, a button should not be used to navigate, which is the purpose of an anchor tag.

<a
data-qa="hover"
href="#"
class="card__buy-button-link"
>
Buy
</a>
</button>
Comment on lines +64 to +72
Copy link

Choose a reason for hiding this comment

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

The button element should not contain an anchor tag. Instead, style the <a> tag to look like a button if you want it to be a link, or use a <button> tag with a click event handler if it's meant to be a button.

Comment on lines +64 to +72
Copy link

Choose a reason for hiding this comment

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

There's a semantic issue here. The <button> element should not contain an <a> link. Instead, you should use either a <button> or an <a>, but not both. If the intention is to navigate, use the <a> tag with button styles. If the intention is to submit a form or trigger a JavaScript action, use the <button> tag.

</div>
</body>
</html>
132 changes: 132 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,135 @@
* {
box-sizing: border-box;
}

body {
margin: 0;
font-family: Roboto, sans-serif;
font-weight: 700;
}

.card {
width: 200px;
border: 1px solid #f3f3f3;
border-radius: 5px;
background-color: #fff;
}
Comment on lines +11 to +16
Copy link

Choose a reason for hiding this comment

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

According to the checklist, the BEM block should not have fixed width and height. Instead, consider making the card width more flexible or controlled by its parent container.


.card__image {
width: 160px;
height: 134px;
margin: 32px 19px 36px;
}

.card__title {
width: 166px;
font-weight: 500;
font-size: 12px;
line-height: 18px;
margin: 0 16px 4px;
color: #060b35;
}

.card__code {
width: fit-content;
font-weight: 400;
font-size: 10px;
line-height: 14px;
color: #616070;
margin: 0 84px 16px 16px;
}

.card__rating {
width: 166px;
height: 16px;
display: flex;
justify-content: space-between;
margin: 0 16px 24px;
}

.stars-star {
width: 16px;
height: 16px;
background-repeat: no-repeat;
background-position: center;
background-image: url(../images/star.svg);

display: inline-block;
}
Comment on lines +50 to +58
Copy link

Choose a reason for hiding this comment

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

The '.stars-star' class has duplicated properties that are also defined in the '.stars .stars-star:nth-child(-n + 4)' class. Consider combining these properties into a single class to follow the DRY principle.


.stars .stars-star:nth-child(-n + 4) {
width: 16px;
height: 16px;
background-repeat: no-repeat;
background-position: center;
background-image: url(../images/star-active.svg);

display: inline-block;
}
Copy link

Choose a reason for hiding this comment

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

The background image for the active stars should be set using a modifier class instead of the nth-child selector to make it more flexible and maintainable. This way, if the number of active stars needs to change, you can just change the class applied to the star elements.


.card__reviews {
font-weight: 400;
font-size: 10px;
line-height: 14px;
align-self: center;
color: #060b35;
}

.card__price-block {
width: 166px;
display: flex;
justify-content: space-between;
height: 18px;
align-items: center;
margin: 0 16px 16px;
}

.card__price-title {
font-weight: 400;
font-size: 12px;
line-height: 18px;
color: #616070;
}

.card__price-amount {
font-weight: 700;
font-size: 16px;
line-height: 18px;
align-self: center;
color: #060b35;
}

.card__buy-button {
width: 166px;
height: 40px;
margin: 0 16px 16px;

border-radius: 5px;
border: #00acdc;
Copy link

Choose a reason for hiding this comment

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

The 'border' property should include the border-style (e.g., solid), not just the color. It seems like there's a mistake here because only the color is specified. According to the checklist, the border property should be written shorthand with all its parts.


background-color: #00acdc;
align-items: center;
}

.card__buy-button-link {
text-transform: uppercase;
text-decoration: none;

font-weight: 700;
font-size: 14px;
line-height: 16px;
align-self: center;
color: #fff;
}

.card__buy-button:hover {
Copy link

Choose a reason for hiding this comment

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

You have specified 'border-radius' and 'border' properties again inside the hover state, but these are already defined outside and don't change on hover. You should remove them from the hover state to follow the DRY principle.

background-color: #fff;

border-radius: 5px;
border: 1px solid #00acdc;
.card__buy-button-link {
font-size: 14px;
line-height: 16px;
color: #00acdc;
}
}
Loading