Homework#1 [Done] #2

TigranKirakosov · 2019-06-27T15:56:18Z

Completed Homework#1

milkiwayRN · 2019-07-02T16:57:44Z

Tigran Kirakosov/index.html

+    <div class="weather-container_widget">
+      <div class="weather-container_first-half">
+        <div class="current-weather__state"><img class="current-weather__state_image" src="/assets/images/icons/sun-ico.png"></div>
+        <div class="current-weather__desc">Sunny</div>


Can you use <p> for elements with text instead of <div>?

Sure, I'll do that!

milkiwayRN · 2019-07-02T16:58:04Z

Tigran Kirakosov/index.html

+      <div class="weather-container_first-half">
+        <div class="current-weather__state"><img class="current-weather__state_image" src="/assets/images/icons/sun-ico.png"></div>
+        <div class="current-weather__desc">Sunny</div>
+        <div class="current-weather__date">Tokyo - Wednesday, July 31</div>


milkiwayRN · 2019-07-02T17:00:31Z

Tigran Kirakosov/index.html

+        <div class="current-weather__date">Tokyo - Wednesday, July 31</div>
+      </div>
+      <div class="weather-container_second-half">
+        <div class="current-weather__temperature">28 &deg;C</div>


milkiwayRN · 2019-07-02T17:01:29Z

Tigran Kirakosov/index.html

+      <div class="weather-container_second-half">
+        <div class="current-weather__temperature">28 &deg;C</div>
+        <div class="current-weather__graph"><img class="current-weather__graph_image" src="/assets/images/graph.png"></div>
+        <div class="current-weather__next-days">


It looks like list of similar elements - can you use <ul> and <li> instead of <div>?

I'll rethink the approach I'm using. Definitely, I should use ul/li here

milkiwayRN · 2019-07-02T17:05:03Z

Tigran Kirakosov/index.html

+        <div class="current-weather__graph"><img class="current-weather__graph_image" src="/assets/images/graph.png"></div>
+        <div class="current-weather__next-days">
+          <div class="day">
+            <img class="day_symb" src="/assets/images/icons/sunny.png"><div class="current-weather__day day-props">Thu</div><div class="day-props">28 &deg;C</div><div class="night-props">18 &deg;C</div>


<img> tag has two required attributes - src and alt https://www.w3schools.com/tags/tag_img.asp

So I should always keep these alt attributes in mind. Though it seemed to be unnecessary for me, and if that is required - so be it

milkiwayRN · 2019-07-02T17:17:50Z

Tigran Kirakosov/style.css

+  align-items: center;
+}
+.current-weather__day {
+  border-bottom: 1px solid #7b1f27;


I see that in general you use the good approach for ordering css properties - it's cool :) but try to not break it - border-bottom shouldn't be before width if you are not sure about some cases - there is good example http://codeguide.academy/html-css.html

Thank you 👍
And thanks for the example - it's a very useful and informative tip!

milkiwayRN · 2019-07-02T17:19:54Z

Tigran Kirakosov/style.css

+.current-weather__day {
+  border-bottom: 1px solid #7b1f27;
+  width: 45px;
+  text-align: center;


the same - text-align should be after any padding

milkiwayRN · 2019-07-02T17:20:48Z

Tigran Kirakosov/style.css

+.weather-container_first-half {
+  display: flex;
+  flex-direction: column;
+  text-align: center;


the same about ordering

milkiwayRN · 2019-07-02T17:24:28Z

Tigran Kirakosov/style.css

+  background-image: url("/assets/images/bg/Tokyo-bg.png");
+  border-radius: 10px;
+  color: white;
+  cursor: default;


just out of curiosity - isn't cursor: default for <article>?

Yes, I did it in order to prevent cursor change on text hovering. I feel like its better for a user not to think about widget like its something common to the rest of the environment, but instead something different. I may be wrong, but for me, it feels right

Ok, actually I don't have any concerns about that 👍

milkiwayRN · 2019-07-02T17:25:00Z

Tigran Kirakosov/style.css

+}
+/* Highier level containers states */
+.weather-container_main {
+  padding: 80px 110px;


the same about ordering

I missed something. What was that previous note about ordering?...
upd: If it's about ordering css props, I'll fix that

yeap - it's about css properties

milkiwayRN · 2019-07-02T19:04:24Z

Tigran Kirakosov/index.html

+      <ul class="weather-container_second-half">
+        <li class="current-weather__temperature"><p>28 &deg;C</p></li>
+        <li class="current-weather__graph"><img class="current-weather__graph_image" src="./assets/images/graph.png" alt="weather graph"></li>
+        <ul class="current-weather__next-days">


yeah, that's what I meant 👍

milkiwayRN · 2019-07-02T19:13:18Z

Tigran Kirakosov/index.html

+        <p class="current-weather__desc">Sunny</p>
+        <p class="current-weather__date">Tokyo - Wednesday, July 31</p>
+      </div>
+      <ul class="weather-container_second-half">


I think this ul is overhead - you have 3 absolutely different items inside (in general list items should be quite similar), but it's rather about semantic than some technical issue. more important - third element of list is <ul> - by specification children of <ul> should be just <li> elements (https://stackoverflow.com/questions/12242041/allowed-child-elements-of-ul)

Yea, the first <ul> looks kinda weird and has no certain reason.
Well, I'll keep in mind the fact <ul> must contain nothing but <li> child elements.

TigranKirakosov added 6 commits June 27, 2019 16:23

Added images and font props

a34bebb

Added a bit of styling

074a21c

Prepared stuff to begin work with flexbox

79c0e70

Done with flexbox tunning

33b31d3

Did some code cleaning

81dc8b8

Rethinked semantic of the markup

8c7b0b9

TigranKirakosov changed the title ~~Первый ПР~~ Tokyo Weather Widget Markup&Layout Jun 27, 2019

TigranKirakosov added 2 commits June 28, 2019 01:00

Changed project directory

94a7b9d

Some minor corrections

b7bf5a7

milkiwayRN suggested changes Jul 2, 2019

View reviewed changes

TigranKirakosov added 2 commits July 2, 2019 22:46

Changes after code review

c2c00bc

Additional microchange

2dba840

milkiwayRN suggested changes Jul 2, 2019

View reviewed changes

More after review changes & fixes for crossbrowser view

75dfe52

milkiwayRN approved these changes Jul 2, 2019

View reviewed changes

TigranKirakosov closed this Jul 3, 2019

TigranKirakosov reopened this Jul 17, 2019

TigranKirakosov changed the title ~~Tokyo Weather Widget Markup&Layout~~ Homework#1 [Done] Aug 7, 2019

Homework#1 [Done] #2

Are you sure you want to change the base?

Homework#1 [Done] #2

Uh oh!

Conversation

TigranKirakosov commented Jun 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TigranKirakosov Jul 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TigranKirakosov commented Jun 27, 2019 •

edited

Loading

TigranKirakosov Jul 2, 2019 •

edited

Loading