Skip to content

Homework#15

Open
AlinaFirsova101 wants to merge 2 commits into
epam-js-jun-2019:masterfrom
AlinaFirsova101:master
Open

Homework#15
AlinaFirsova101 wants to merge 2 commits into
epam-js-jun-2019:masterfrom
AlinaFirsova101:master

Conversation

@AlinaFirsova101
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@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

Comment thread Alina Firsova/flex/flex.html Outdated
@@ -0,0 +1,39 @@
<!DOCTYPE HTML>
Copy link
Copy Markdown

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'

Comment thread Alina Firsova/grid/grid.html Outdated
@@ -0,0 +1,43 @@
<!DOCTYPE HTML>
Copy link
Copy Markdown

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

Comment thread Alina Firsova/flex/flex.html Outdated
<meta name="viewport" content="width=device-width,
initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
</head>
<body class="background">
Copy link
Copy Markdown

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

Comment thread Alina Firsova/flex/flex.html Outdated
<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>
Copy link
Copy Markdown

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

Comment thread Alina Firsova/flex/style.css Outdated
}


}
Copy link
Copy Markdown

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;
}
}

`

Comment thread Alina Firsova/flex/style.css Outdated


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

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

Comment thread Alina Firsova/flex/style.css Outdated
display: flex;
justify-content: center;
align-items: center;
flex-direction:column;
Copy link
Copy Markdown

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;

Comment thread Alina Firsova/flex/style.css Outdated
background-size: contain;
width: 205px;
height: 205px;
height: 205px;
Copy link
Copy Markdown

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