Skip to content

Conversation

@AlinaFirsova101
Copy link

No description provided.

Copy link

@aleksandrbelik aleksandrbelik left a comment

Choose a reason for hiding this comment

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

Need to format css files to avoid blank string and useless tabs
Grid variant works correct only in Chrome unfortunately

@@ -0,0 +1,39 @@
<!DOCTYPE HTML>

Choose a reason for hiding this comment

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

@AlinaFirsova It's better to keep main html file named as 'index.html'

@@ -0,0 +1,43 @@
<!DOCTYPE HTML>

Choose a reason for hiding this comment

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

@AlinaFirsova Same issue with file name

<meta name="viewport" content="width=device-width,
initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
</head>
<body class="background">

Choose a reason for hiding this comment

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

@AlinaFirsova It's better to avoid such common class names

<div class="main-container__element"><img src="img\half_moon.png"></div>
<div class="main-container__week-bottom">
<div class="main-container__week">
<div class="main-container__day"><div class="main-container__name">MON</div><div class="main-container__image-div"><img height="80%" src="img\rainy_small.png"></div><div class="main-container__value">22°</div></div>

Choose a reason for hiding this comment

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

@AlinaFirsova Please set image height in styles

}


}

Choose a reason for hiding this comment

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

@AlinaFirsova you can set styles for main-container by defult (for small screens for ex.) and then add media only for bigger cases
`
.main-container {
height: 615px;
}
@media screen and (min-width: 615px) {
.main-container {
width: 615px;
height: 410px;
}
}

`



.main-container__chicago{
background-image:url(img/chicago_back.png);

Choose a reason for hiding this comment

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

@AlinaFirsova It's much more better to set background color instead of image filled with one color

display: flex;
justify-content: center;
align-items: center;
flex-direction:column;

Choose a reason for hiding this comment

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

@AlinaFirsova You can extract several properties to separate class to avoid suplicating them.
These properties looks like common for a few blocks:
background-repeat: no-repeat; width: 205px; height: 205px; display: flex; justify-content: center; align-items: center; flex-direction:column;

background-size: contain;
width: 205px;
height: 205px;
height: 205px;

Choose a reason for hiding this comment

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

@AlinaFirsova duplicated property

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.

2 participants