add /src #1 #5055

Nikola57 · 2024-12-11T17:46:30Z

Requirements:

reset the browser's default margins
card width is 200px including border
use images from src/images
change link styles on :hover
add data-qa="card" attribute to the card block
add data-qa="hover" attribute to the link BUY
background-image: url() should be relative to the index.scss. So it should start with ../images.
Rewrite the stars block from the Stars task with SCSS and use it
Find the required font on google fonts and use.

Important note: In this task, you are allowed to link *.scss files directly in HTML <link> tags using href attribute.
This is possible because we use the Parcel library to bundle your solution's source code.

mateacademy-ai-mentor

Thank you for your hard work on this project! 😊 However, there are a few critical issues that need to be addressed before we can approve the pull request:

BEM Blocks: The task requires two BEM blocks, card and stars, each in their own file. It seems the stars block is not clearly separated, which is a core requirement.
SCSS Compilation: The HTML file links directly to SCSS files, which browsers cannot interpret. You need to compile the SCSS into CSS and link to the resulting CSS file.
Variable Typos: There are multiple instances of the typo $main-font-famaly instead of $main-font-family. This needs to be corrected across all SCSS files to ensure styles are applied correctly.
Import Path Typos: The import paths contain typos such as veriables instead of variables and reviws instead of reviews. These should be corrected to prevent import errors.
Missing Units: Some margin properties are missing units, which can lead to unexpected behavior. Ensure all values have appropriate units.
Google Fonts: Importing Google Fonts directly in SCSS is not recommended. It's better to include the font link in the HTML file for better performance.
Cross-Origin Attribute: The crossorigin attribute is missing in the preconnect link to Google Fonts, which can improve performance.

Please address these issues to meet the task requirements. Once these changes are made, we can proceed with the approval. Keep up the great work, and don't hesitate to reach out if you need any help! 🌟

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2024-12-11T17:48:54Z

src/index.html

+    <link
+      rel="stylesheet"
+      href="./style.scss"
+    />


The link tag is referencing a SCSS file (./style.scss). Browsers cannot directly interpret SCSS files. You need to compile the SCSS into CSS and link to the resulting CSS file instead.

mateacademy-ai-mentor · 2024-12-11T17:48:54Z

src/index.html

+    <link
+      rel="preconnect"
+      href="https://fonts.gstatic.com"
+    />


The 'crossorigin' attribute is missing in this preconnect link to 'https://fonts.gstatic.com'. Adding 'crossorigin' can improve performance by allowing the browser to make the connection early.

mateacademy-ai-mentor · 2024-12-11T17:48:55Z

src/style.scss

@@ -0,0 +1,10 @@
+@import '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';


Importing Google Fonts directly in SCSS is not recommended. It's better to include the font link in the HTML file to ensure fonts are loaded correctly and efficiently.

mateacademy-ai-mentor · 2024-12-11T17:48:55Z

src/style.scss

@@ -0,0 +1,10 @@
+@import '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';
+@import './veriables';


There is a typo in the import path: 'veriables' should be 'variables'. Ensure the file name matches the actual file.

mateacademy-ai-mentor · 2024-12-11T17:48:55Z

src/style.scss

+@import './styles/blocks/contentText';
+@import './styles/blocks/contentCode';
+@import './styles/index';
+@import './styles/blocks/reviws';


There is a typo in the import path: 'reviws' should be 'reviews'. Ensure the file name matches the actual file.

mateacademy-ai-mentor · 2024-12-11T17:48:56Z

src/styles/blocks/main.scss

+}
+
+.contentInformacion {
+  margin: 0 16 16;


The 'margin' property values are missing units. Ensure that each value has a unit (e.g., 'px', 'em'). For example, 'margin: 0 16px 16px;'.

mateacademy-ai-mentor · 2024-12-11T17:48:56Z

src/styles/blocks/main.scss

+  top: 1px;
+  left: 1px;
+  border-radius: 5px;
+  border: 1px;


The 'border' property is redundant as it is immediately overridden by the next 'border' property. You can remove this line to avoid confusion.

mateacademy-ai-mentor · 2024-12-11T17:48:56Z

src/styles/blocks/price.scss

+    flex-direction: row;
+    width: 31px;
+    height: 18px;
+    font-family: $main-font-famaly;


There is a typo in the variable name '$main-font-famaly'. It should be '$main-font-family'. Ensure that the variable is correctly defined and imported.

mateacademy-ai-mentor · 2024-12-11T17:48:56Z

src/styles/blocks/reviws.scss

+    display: flex;
+    justify-content: right;
+    align-items: end;
+    font-family: $main-font-famaly;


There is a typo in the variable name '$main-font-famaly'. It should be '$main-font-family'. Ensure that the variable is correctly defined and imported.

mateacademy-ai-mentor · 2024-12-11T17:48:56Z

src/veriables.scss

@@ -0,0 +1,3 @@
+$main-text-color: #060b35;
+$main-button-color: #00acdc;
+$main-font-famaly: Roboto;


There is a typo in the variable name '$main-font-famaly'. It should be '$main-font-family'. Update this variable name and ensure all references in your SCSS files are consistent.

mateacademy-ai-mentor

The Pull Request has been reviewed, and while there are some issues that need attention, it is approved to help you move forward with your learning journey. 🚀

Review Summary:

SCSS Compilation: Ensure that SCSS files are compiled into CSS, as browsers cannot interpret SCSS directly. This is crucial for the styling to work correctly.
Google Fonts: The crossorigin attribute should be set to anonymous for better performance when fetching resources from Google Fonts.
Image Path Typo: Correct the typo in the background image path from .jpag to .jpg to ensure the image loads correctly.
Positioning Properties: The top and left properties require a positioning context to be effective. Consider adding position: relative; or another appropriate positioning property.
Unused and Redundant Classes: The .body class is defined but not used. Since the body element is already styled, consider removing this redundant class to avoid confusion.

Your effort in addressing previous issues is commendable, and the code now meets the task requirements. However, please review the comments to improve your code further. Addressing these critical issues in the future will help you avoid similar problems and enhance your coding skills. Keep up the good work and continue learning from these experiences! 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-12-15T19:58:32Z

src/index.html

@@ -11,8 +12,67 @@
      rel="stylesheet"
      href="./styles/index.scss"


Browsers cannot interpret SCSS files directly. You need to compile index.scss into a CSS file and link to the resulting CSS file instead.

mateacademy-ai-mentor · 2024-12-15T19:58:32Z

src/index.html

+    <link
+      rel="preconnect"
+      href="https://fonts.gstatic.com"
+      crossorigin=" "


The crossorigin attribute is set with an empty value. It should be set to anonymous to improve performance when fetching resources from Google Fonts.

mateacademy-ai-mentor · 2024-12-15T19:58:32Z

src/style.css

+  height: 134px;
+  background-position: center;
+  background-size: 100% 100%;
+  background-image: url("./images/imac.jpag");


There is a typo in the background image path. The file extension should be .jpg instead of .jpag.

mateacademy-ai-mentor · 2024-12-15T19:58:33Z

src/style.css

+  background-image: url("./images/star-active.svg");
+}
+
+.body {


The .body class is defined but not used in the HTML file. Consider removing it if it's unnecessary, as the body element is already styled.

mateacademy-ai-mentor · 2024-12-15T19:58:33Z

src/styles/blocks/card__code.scss

+  top: 247px;
+  left: 17px;


The top and left properties will have no effect unless the element has a positioning context. Consider adding position: relative; or another positioning property if needed.

mateacademy-ai-mentor · 2024-12-15T19:58:33Z

src/styles/blocks/card__photo.scss

+  height: 134px;
+  background-position: center;
+  background-size: 100% 100%;
+  background-image: url('./images/imac.jpag');


There is a typo in the background image path. The file extension should be .jpg instead of .jpag.

mateacademy-ai-mentor · 2024-12-15T19:58:33Z

src/styles/blocks/card__text.scss

+  top: 207px;
+  left: 17px;


The top and left properties will have no effect unless the element has a positioning context. Consider adding position: relative; or another positioning property if needed.

mateacademy-ai-mentor · 2024-12-15T19:58:33Z

src/styles/blocks/main.scss

+  top: 1px;
+  left: 1px;


The top and left properties will have no effect unless the element has a positioning context. Consider adding position: relative; or another positioning property if needed.

mateacademy-ai-mentor · 2024-12-15T19:58:33Z

src/styles/blocks/stars.scss

+  &-star {
+    width: 16px;
+    height: 16px;
+    left: 80px;


The left property will have no effect unless the element has a positioning context. Consider adding position: relative; or another positioning property if needed.

mateacademy-ai-mentor · 2024-12-15T19:58:33Z

src/styles/index.scss

@@ -1,3 +1,3 @@
-body {
+.body {


The .body class seems unnecessary as the body element is already styled in main.scss. Consider removing this redundant class.

mateacademy-ai-mentor suggested changes Dec 11, 2024

View reviewed changes

Fixed BEM and gramatic

c24741e

Nikola57 force-pushed the develop branch from cb2659a to c24741e Compare December 15, 2024 18:20

Nikola57 added 3 commits December 15, 2024 19:35

Fixed BEM scss => css

db789f0

fix img card__photo

9406b13

fix img card__photo in scss mate-academy#2

5fcce0c

Nikola57 requested a review from mateacademy-ai-mentor December 15, 2024 19:56

mateacademy-ai-mentor approved these changes Dec 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add /src #1 #5055

add /src #1 #5055

Nikola57 commented Dec 11, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

mateacademy-ai-mentor Dec 15, 2024

		@@ -0,0 +1,10 @@
		@import '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';

		@@ -0,0 +1,10 @@
		@import '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';
		@import './veriables';

		@@ -11,8 +12,67 @@
		rel="stylesheet"
		href="./styles/index.scss"

		@@ -1,3 +1,3 @@
		body {
		.body {

add /src #1 #5055

Are you sure you want to change the base?

add /src #1 #5055

Conversation

Nikola57 commented Dec 11, 2024

Requirements:

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment