stars making #4875

RiTAnd · 2024-09-30T16:50:56Z

Yellow stars are added with container modifier + pseudo-selector (NO extra classes)
Each BEM block has its own separate file
All Typical Mistakes from BEM lesson theory are checked.
Code follows all the Code Style Rules ❗️

mateacademy-ai-mentor

Hey there! 👋 It looks like we need to address a few important layout and code quality issues before we can move forward. Using fixed sizes and tag names for styling might cause some trouble down the line, especially when it comes to responsiveness and maintainability. Also, don't forget to add those empty lines for better readability and to adjust your indentation for a cleaner look. Keep up the good work, and with a bit of tweaking, your code will be top-notch! 💪

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

mateacademy-ai-mentor · 2024-09-30T16:51:22Z

src/index.html

@@ -13,6 +13,52 @@
    />
  </head>
  <body>


The tag should be indented with 2 spaces from the opening tag to maintain proper nesting structure.

mateacademy-ai-mentor · 2024-09-30T16:51:22Z

src/index.html

@@ -13,6 +13,52 @@
    />


The attributes of the tag should be on separate lines with 2-space indentation for consistency and readability, as per the checklist guidelines.

mateacademy-ai-mentor · 2024-09-30T16:51:22Z

src/index.html

+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+    </ul>


Add an empty line between sibling blocks of HTML to improve readability. There should be an empty line before the second
element.

mateacademy-ai-mentor · 2024-09-30T16:51:23Z

src/index.html

+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+    </ul>


Add an empty line between sibling blocks of HTML to improve readability. There should be an empty line before the third
element.

mateacademy-ai-mentor · 2024-09-30T16:51:23Z

src/index.html

+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+    </ul>


Add an empty line between sibling blocks of HTML to improve readability. There should be an empty line before the fourth
element.

mateacademy-ai-mentor · 2024-09-30T16:51:23Z

src/index.html

+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+    </ul>


Add an empty line between sibling blocks of HTML to improve readability. There should be an empty line before the sixth
element.

mateacademy-ai-mentor · 2024-09-30T16:51:23Z

src/style.css

+html {
+  margin: 0;
+  padding: 0;
+}


Don't use tag names for styling (except html and body). You can remove these styles as they are typically default for browsers, or if needed, apply them to the body tag instead.

mateacademy-ai-mentor · 2024-09-30T16:51:23Z

src/style.css

+.stars {
+  display: flex;
+  position: relative;
+  width: 96px;
+  top: 22px;
+  left: 22px;


Don't fix container size (if there is no such a requirement). Setting a fixed width may cause issues with responsiveness. Consider using max-width or a more responsive size unit like percentages or viewport units if needed.

mateacademy-ai-mentor · 2024-09-30T16:51:23Z

src/style.css

+.stars {
+  display: flex;
+  position: relative;
+  width: 96px;


Remember to use fallback fonts - alternative font-family in case the main one doesn't work. Although not directly visible in this snippet, ensure that your font-family declarations throughout your CSS include fallback options.

mateacademy-ai-mentor · 2024-09-30T16:51:23Z

src/style.css

+.stars--1__star:nth-child(1) {
+  top: 42px;
+  left: 22px;
+}
+
+.stars--1__star:nth-child(2) {
+  top: 62px;
+  left: 42px;
+}
+
+.stars--1__star:nth-child(3) {
+  top: 82px;
+  left: 62px;
+}
+
+.stars--1__star:nth-child(4) {
+  top: 102px;
+  left: 82px;
+}
+
+.stars--1__star:nth-child(5) {
+  top: 122px;
+  left: 102px;
+}


It looks like you're trying to position elements individually, which could lead to maintenance issues in the future. Consider using a more dynamic approach such as flexbox or grid layout to position your stars. This will make your code more maintainable and responsive.

mateacademy-ai-mentor

Great job on the Pull Request! There are a few minor issues to address, such as the BEM notation and some styling best practices, but these don't critically impact the functionality or security of your code. Keep up the good work, and consider these points for future improvements to enhance the quality of your code! 😊👍

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

mateacademy-ai-mentor · 2024-09-30T17:34:40Z

src/index.html

+    <ul class="stars stars--0">
+
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+
+    </ul>
+
+    <ul class="stars stars--1">
+
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+
+    </ul>
+
+    <ul class="stars stars--2">
+
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+
+    </ul>
+
+    <ul class="stars stars--3">
+
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+
+    </ul>
+
+    <ul class="stars stars--4">
+
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+
+    </ul>
+
+    <ul class="stars stars--5">
+
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>
+      <li class="stars__star"></li>


Add empty lines between multiline sibling blocks of HTML. There should be an empty line between each
block to improve readability.

mateacademy-ai-mentor · 2024-09-30T17:34:40Z

src/style.css

+.stars {
+  display: flex;
+  position: relative;
+  width: 96px;
+  top: 22px;
+  left: 22px;


You're setting fixed sizes with 'width', 'top', and 'left' properties. If there is no such requirement, consider using sizes that are relative to the content or parent element to prevent overflow or accidental scroll bars.

mateacademy-ai-mentor · 2024-09-30T17:34:40Z

src/style.css

+.stars__star {
+  display: inline-block;
+  margin-right: 5px;
+  width: 16px;
+  height: 16px;
+  background-image: url(images/star.svg);
+  background-size: contain;
+  background-repeat: no-repeat;


It's good to see that you have a separate class for styling stars. However, avoid using the 'margin-right' property on the last child element as it can cause unnecessary spacing. Consider using the ':not(:last-child)' pseudo-class to apply the margin to all except the last child.

mateacademy-ai-mentor · 2024-09-30T17:34:40Z

src/style.css

+.stars--1__star:nth-child(1) {
+  top: 42px;
+  left: 22px;
+}
+
+.stars--1__star:nth-child(2) {
+  top: 62px;
+  left: 42px;
+}
+
+.stars--1__star:nth-child(3) {
+  top: 82px;
+  left: 62px;
+}


The classes '.stars--1__star' seem to be a typo or incorrect BEM notation. If these are meant to be modifiers, they should be using the single dash notation like '.stars--1 .stars__star' and not double underscore. Also, ensure that you're not styling by tag names as per the checklist.

stars making

4dd6d2d

mateacademy-ai-mentor suggested changes Sep 30, 2024

View reviewed changes

correction error

1c4f6a6

RiTAnd requested a review from mateacademy-ai-mentor September 30, 2024 17:34

mateacademy-ai-mentor approved these changes Sep 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stars making #4875

stars making #4875

RiTAnd commented Sep 30, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

mateacademy-ai-mentor Sep 30, 2024

@@ @@ -13,6 +13,52 @@ @@
                   />
                 </head>
                 <body>

stars making #4875

Are you sure you want to change the base?

stars making #4875

Conversation

RiTAnd commented Sep 30, 2024

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

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