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.

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

839 implement the who we are page #891

Merged
merged 25 commits into from
Nov 26, 2024

Conversation

parsival2022
Copy link
Collaborator

@parsival2022 parsival2022 commented Nov 13, 2024

image

image

image

@parsival2022 parsival2022 linked an issue Nov 13, 2024 that may be closed by this pull request
@Lvyshnevska Lvyshnevska changed the base branch from master to develop November 13, 2024 17:37
@parsival2022 parsival2022 removed the request for review from mehalyna November 13, 2024 18:10
}

.about-us-section-content__header {
margin-top: 80px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a redundant margin-top that doubles the 80px spacing. The parent class .about-us-section-main already has padding: 80px, which is enough.

}

.about-us-section-content {
padding: 40px 104px 48px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are redundant paddings. The parent class .about-us-section-main already provides the necessary spacing through its padding and gap properties.

.about-us-section-content__header {
color: var(--about-us-header-color);
margin-bottom: 24px;
margin-top: 40px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are redundant margins. The necessary spacing is provided through the padding and gap in .about-us-section-main class.

justify-content: center;
align-items: center;
overflow: hidden;
padding-bottom: 40px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is redundant padding-bottom. The necessary spacing is provided through the padding in .about-us-section-main class.

width: 375px;
padding: 0 15px;
display: flex;
justify-content: space-between;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a redundant justify-content property. It has no effect on mobile and tablet screens. And it is only applied for screens > 1200px in media query.

min-width: 100vw;
background: var(--about-us-background-color);
justify-content: center;
align-items: center;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are redundant justify-content and align-items properties that have no effect.

@YanZhylavy YanZhylavy merged commit 8dfde19 into develop Nov 26, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Redesign] Implement the "Хто ми" (Who We Are) Page
4 participants