Develop

[Artemida1609]

• DEMO LINK
• TEST REPORT LINK

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

• there 2 BEM blocks card and stars each in it own file
• SCSS Nesting is used for elements, modifiers and pseudo-classes
• SCSS Variables are used for main values and placed in a separate file
• all stars--N modifiers work as expected (Highlight first N stars)
• Code follows all the Code Style Rules ❗️

[mateacademy-ai-mentor]

Hey there! It looks like there are a few critical issues that need your attention before we can move forward. 😊 The active stars should use a modifier class for flexibility, and properties shouldn't be duplicated to keep the code DRY. Also, make sure the border property is complete, and correct the way you're using buttons and links. Lastly, check the file extensions for images and CSS files. Once you address these points, your code will be in great shape! Keep up the good work, and I'm looking forward to seeing the improved version! 👍

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

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

Great effort on the work you've done so far! However, there are a few critical issues that need to be addressed before we can move forward. The use of incorrect BEM naming and the presence of a button element around an anchor tag are significant concerns that could affect the maintainability and functionality of the code. Also, the fixed dimensions on the BEM block could limit flexibility. Please take a look at these points and make the necessary changes. Keep up the good work, and you'll have this polished up in no time! 😊🛠️

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

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

The .stars .stars-star:nth-child(-n + 4) selector does not follow the BEM methodology. Instead of modifying the element based on a parent class, you should use a modifier class for the star element itself. For example, .stars-star--active could be used to represent an active star.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

The nested selector .card__buy-button-link inside the :hover pseudo-class is not recommended in SCSS. Instead, you should use the & parent selector to keep the nesting within the .card__buy-button block scope. For example, &:hover &__buy-button-link { ... }.

[mateacademy-ai-mentor]

Great effort on the Pull Request! 👍 However, please address the following issues in the future:

1. Use correct SCSS nesting for the hover state.
2. Include all necessary assets or adjust the code accordingly.
3. Correct the semantic use of <button> and <a> elements.
4. Use appropriate tags for non-heading content.
5. Remember to compile SCSS to CSS and link the CSS file in your HTML.

These points will enhance the quality and maintainability of your code. Keep up the good work and happy coding! 😊

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

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

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.

[mateacademy-ai-mentor]

The class .stars-star is modified by :nth-child(-n + 4) to change the image, which suggests that these are active stars, but there is no star-active.svg provided in the changed files. Make sure to include all necessary assets if they are meant to be part of the pull request. If star-active.svg is not meant to be different from star.svg, consider removing this rule to avoid confusion.

[mateacademy-ai-mentor]

The nested SCSS inside .card__buy-button:hover should be written as '&:hover' directly inside the .card__buy-button block. Nested rules like this won't compile correctly in standard SCSS and will lead to a syntax error. This is how it should be structured:

.card__buy-button {
...styles...
&:hover {
...hover styles...
.card__buy-button-link {
...link styles...
}
}
}