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

add task solution #3493

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

add task solution #3493

wants to merge 3 commits into from

Conversation

AlinaChukh
Copy link

@AlinaChukh AlinaChukh commented Aug 18, 2023 •
edited
Loading

xiimiik
xiimiik suggested changes Aug 18, 2023
View reviewed changes
src/index.html Outdated
Comment on lines 19 to 25
<div class="stars stars--1">
<div class="stars__star yellow--color"></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.

read the description more carefully, activity styles must be provided with a modifier
tip: use :nth-child in css
image

src/style.css Outdated
Comment on lines 5 to 8
.main {
width: 96px;
margin: auto;
}
Copy link

Choose a reason for hiding this comment

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

delete it from the html and css

src/style.css Outdated
Comment on lines 10 to 13
.stars {
display: flex;
justify-content: space-between;
}
Copy link

Choose a reason for hiding this comment

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

just flex, add indents using margin-right, and don't forget to remove the margin from the last element

Suggested change
.stars {
display: flex;
justify-content: space-between;
}
.stars {
display: flex;
}

src/style.css Outdated
Comment on lines 21 to 23
.yellow--color {
background: url(../src/images/star-active.svg) no-repeat;
}
Copy link

Choose a reason for hiding this comment

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

delete

src/style.css Outdated
Comment on lines 15 to 19
.stars__star {
background: url(../src/images/star.svg) no-repeat;
width: 16px;
height: 16px;
}
Copy link

Choose a reason for hiding this comment

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

add background-position: center;

mykhalenych
mykhalenych suggested changes Aug 18, 2023
View reviewed changes
readme.md Outdated
@@ -1,7 +1,7 @@
# Stars block
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://<AlinaChukh>.github.io/layout_stars/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [DEMO LINK](https://<AlinaChukh>.github.io/layout_stars/)
- [DEMO LINK](https://AlinaChukh.github.io/layout_stars/)

readme.md Outdated
- [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://<AlinaChukh>.github.io/layout_stars/)
- [TEST REPORT LINK](https://<AlinaChukh>.github.io/layout_stars/report/html_report/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [TEST REPORT LINK](https://<AlinaChukh>.github.io/layout_stars/report/html_report/)
- [TEST REPORT LINK](https://AlinaChukh.github.io/layout_stars/report/html_report/)

src/style.css Outdated
display: flex;
margin: 0;
flex-direction: column;
align-items: center;
Copy link
Contributor

Choose a reason for hiding this comment

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

delete all styles from body, keep only margin: 0;

src/style.css Outdated
margin-right: 4px;
}

.stars--1 > .stars__star:nth-child(-n + 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.stars--1 > .stars__star:nth-child(-n + 1) {
.stars--1 > .stars__star:nth-child(-n + 1),
.stars--2 > .stars__star:nth-child(-n + 2),
.stars--3 > .stars__star:nth-child(-n + 3),
... {
}

src/style.css Outdated

.stars--1 > .stars__star:nth-child(-n + 1) {
background: url(../src/images/star-active.svg) no-repeat;
background-position: center;
Copy link
Contributor

Choose a reason for hiding this comment

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

you already added this style in .stars__star, delete here

andriy-shymkiv
andriy-shymkiv approved these changes Aug 18, 2023
View reviewed changes
Copy link

@andriy-shymkiv andriy-shymkiv left a comment

Choose a reason for hiding this comment

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

good job, just fix indent

src/index.html
<title>Stars</title>
<link rel="stylesheet" href="./style.css">
</head>
<body>
<h1>Stars</h1>

Copy link

Choose a reason for hiding this comment

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

Suggested change

src/index.html
<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.

Suggested change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andriy-shymkiv andriy-shymkiv andriy-shymkiv approved these changes

@xiimiik xiimiik Awaiting requested review from xiimiik

@mykhalenych mykhalenych Awaiting requested review from mykhalenych

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AlinaChukh @mykhalenych @andriy-shymkiv @xiimiik