completed weeks 1,2,3

[saeed6617]

No description provided.

[tekul]

Looks good Saeed 👍 . You don't seem to have had any serious issues with this module and your work is well organised. If you find you are using <br> tags for the layout of your page then you should probably be using CSS instead. For example, this specification (https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-br-element) recommends that you only use them "only for line breaks that are actually part of the content, as in poems or addresses". This is a very minor issue though.

[tekul]

Missing the "h" in "https". It's a good idea to test your changes before committing (e.g. by clicking on the link here).

[tekul]

Also links look best if they don't include trailing space, so keep the space outside the <a> tag.

Let's meet at the <a href="https://goo.gl/maps/aza4h9nUBhn">iCafe</a> in the Merchant City</a>.

[tekul]

You don't actually need display: flex; on each one of these since it's already included in the class countries (which each element also has). It might seem clearer this way though.

[tekul]

What types are the email and phone aren't supposed to be according to the instructions?

[tekul]

You shouldn't need to use <br> tags. Presentation should generally be taken care of by CSS.

[saeed6617]

Got it. Many thanks, Saeed

…

On Wed, Mar 6, 2019 at 15:56 ___ ***@***.***> wrote: ***@***.**** commented on this pull request. Looks good Saeed 👍 . You don't seem to have had any serious issues with this module and your work is well organised. If you find you are using <br> tags for the layout of your page then you should probably be using CSS instead. For example, this specification ( https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-br-element) recommends that you only use them "only for line breaks that are actually part of the content, as in poems or addresses". This is a very minor issue though. ------------------------------ In week-1/2-html-attributes/index.html <#63 (comment)> : > @@ -23,15 +23,14 @@ <div class="message"> <div class="message__author">Luke</div> <p class="message__content"> - Let's meet at the iCafe in Merchant City. - https://goo.gl/maps/aza4h9nUBhn + Let's meet at the<a href="ttps://goo.gl/maps/aza4h9nUBhnh"> iCafe</a> in Merchant City. Missing the "h" in "https". It's a good idea to test your changes before committing (e.g. by clicking on the link here). ------------------------------ In week-1/2-html-attributes/index.html <#63 (comment)> : > @@ -23,15 +23,14 @@ <div class="message"> <div class="message__author">Luke</div> <p class="message__content"> - Let's meet at the iCafe in Merchant City. - https://goo.gl/maps/aza4h9nUBhn + Let's meet at the<a href="ttps://goo.gl/maps/aza4h9nUBhnh"> iCafe</a> in Merchant City. Also links look best if they don't include trailing space, so keep the space outside the <a> tag. Let's meet at the <a href="https://goo.gl/maps/aza4h9nUBhn">iCafe</a> in the Merchant City</a>. ------------------------------ In week-2/13-align-items/flexbox.css <#63 (comment)> : > @@ -2,3 +2,23 @@ display: flex; } /* Add your own CSS code below */ +.countries--first { + display: flex; You don't actually need display: flex; on each one of these since it's already included in the class countries (which each element also has). It might seem clearer this way though. ------------------------------ In week-3/22-checkout/index.html <#63 (comment)> : > + </table> + </div> + + <div class="checkout"> + <h2 class="checkout__title">Checkout</h2> + <form method="GET" id="checkout-form" class="checkout__form"> + <label for="customer-name">Name</label> + <input id="customer-name" type="text" name="customer-name" /> + <br /> + + <label for="customer-email">Email</label> + <input id="customer-email" type="number" name="customer-email" /> + <br /> + + <label for="customer-phone">Phone number</label> + <input id="customer-phone" type="number" name="customer-phone" /> What types are the email and phone aren't supposed to be according to the instructions? ------------------------------ In week-3/22-checkout/index.html <#63 (comment)> : > + <p>CodeYourFuture t-shirt, black</p> + </div> + </td> + <td>Medium</td> + <td>1</td> + <td>£10.00</td> + </tr> + </table> + </div> + + <div class="checkout"> + <h2 class="checkout__title">Checkout</h2> + <form method="GET" id="checkout-form" class="checkout__form"> + <label for="customer-name">Name</label> + <input id="customer-name" type="text" name="customer-name" /> + <br /> You shouldn't need to use <br> tags. Presentation should generally be taken care of by CSS. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#63 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AtEhzy2wIKDjgJG5Nuw5T20sX9iA2Zieks5vT-UfgaJpZM4bVyUj> .