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

solution #4961

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

solution #4961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a9f0cd9
solution
VitaliiHutsalo Oct 16, 2024
dca60e6
solution
VitaliiHutsalo Oct 16, 2024
1cef9f7
solution
VitaliiHutsalo Oct 16, 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
6 changes: 3 additions & 3 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Implement the [Stars Block](https://www.figma.com/file/ojkArVazq7vsX0nbpn9CxZ/Moyo-%2F-Catalog-(ENG)?node-id=11325%3A2960) used in a card and catalog.

Hold `Alt` key (`Option` on MacOS) to measure distances in Figma.
Hold `Alt` key (`Option` on MacOS) to measure distances in Figma.

> Here are the [Layout Tasks Instructions](https://mate-academy.github.io/layout_task-guideline)

Expand All @@ -22,8 +22,8 @@ Hold `Alt` key (`Option` on MacOS) to measure distances in Figma.

❗️ Replace `<your_account>` with your Github username and copy the links to `Pull Request` description:

- [DEMO LINK](https://<your_account>.github.io/layout_stars/)
- [TEST REPORT LINK](https://<your_account>.github.io/layout_stars/report/html_report/)
- [DEMO LINK](https://VitaliiHutsalo.github.io/layout_stars/)
- [TEST REPORT LINK](https://VitaliiHutsalo.github.io/layout_stars/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
74 changes: 73 additions & 1 deletion src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<!--#region head-->
<!doctype html>
<html lang="en">
<head>
Expand All @@ -12,7 +13,78 @@
href="./style.css"
/>
Copy link

Choose a reason for hiding this comment

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

HTML Formatting: The attributes of your link tag are not correctly formatted. Each attribute should start on a new line with a 2-space indentation relative to the tag.

</head>
<!--#endregion-->
<body>
<h1>Stars</h1>
<div class="stars stars--0">
<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>
Copy link

Choose a reason for hiding this comment

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

HTML Content: It would be better to use semantic tags instead of div for the stars blocks. Consider using a semantic tag like section or article that would better describe the content.

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, you should add empty lines between multiline sibling blocks of HTML to simplify reading. However, you have added empty lines between child elements, which is not recommended. You should remove the empty lines between the div elements with the class stars__star.


<div class="stars stars--1">
<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="stars stars--2">
<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="stars stars--3">
<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="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="stars stars--5">
<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>
</body>
</html>
45 changes: 45 additions & 0 deletions src/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,46 @@
/* add styles here */
html,
body {
margin: 0;
padding: 0;
box-sizing: border-box;
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, you should avoid using the * selector for styling, as it impacts performance. Although this is a minor issue, it's a good practice to style only the elements that require it.

}
Copy link

Choose a reason for hiding this comment

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

Avoid using the * selector for styling. It impacts performance as it affects all elements. Try to set styles only for elements that require them.

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 checklist item that suggests not to use tag names for styling, except for html and body.


.stars {
display: flex;
}

.stars__star {
display: flex;
background-image: url(images/star.svg);
width: 16px;
height: 16px;
border-radius: 0.5px;
background-repeat: no-repeat;
background-position: center;
margin-right: 4px;
Copy link

Choose a reason for hiding this comment

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

Remember to use fallback fonts. It's an alternative font-family in case the main one doesn't work. You can specify a list of fonts separated by commas. The browser will use the first font in the list available on the user's computer.

}
Copy link

Choose a reason for hiding this comment

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

Be consistent with your margins. It seems you are applying the margin-right property to all .stars__star elements. This could potentially lead to margin collapse. Consider applying margins only to the top or bottom, but not both.


.stars--1 :first-child {
background-image: url(./images/star-active.svg);
}
Comment on lines +24 to +26
Copy link

Choose a reason for hiding this comment

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

The path to the images is not consistent. Here you're using ./images/star-active.svg, while in other places you're using images/star-active.svg. It's not an error, but it's better to keep the code consistent.


.stars--2 :nth-child(-n + 2) {
background-image: url(images/star-active.svg);
}

.stars--3 :nth-child(-n + 3) {
background-image: url(images/star-active.svg);
}

.stars--4 :nth-child(-n + 4) {
background-image: url(images/star-active.svg);
}

.stars--5 :nth-child(-n + 1),
.stars--5 :nth-child(-n + 2),
.stars--5 :nth-child(-n + 3),
.stars--5 :nth-child(-n + 4),
.stars--5 :nth-child(-n + 5) {
Comment on lines +40 to +44
Copy link

Choose a reason for hiding this comment

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

There seems to be a mistake in your CSS selectors. The :nth-child(-n + 1) selector in the .stars--5 rule will only select the first child, but you probably intended to select all children. To select all children, you can simply use the :nth-child(n) selector. Also, there's no need to repeat the same selector for each child. You can combine them into a single rule: .stars--5 :nth-child(n).

background-image: url(images/star-active.svg);
}
Comment on lines +40 to +46
Copy link

Choose a reason for hiding this comment

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

You can simplify the CSS rule for .stars--5 by using :nth-child(-n + 5) only. There's no need to repeat it for each child.

Copy link

Choose a reason for hiding this comment

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

It's a good practice to avoid fixing container size if there is no such a requirement. Let the content size dictate it. This can help to avoid overflow or accidental scroll bar.

Loading