glasgow-class5/salem-dayfan-html/css-week2 by sSalem123 · Pull Request #222 · CodeYourFuture/HTML-CSS-Module-Project

sSalem123 · 2021-06-11T14:13:42Z

Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in HOW_TO_MARK.md in the root of this repository

Your Details

Your Name:
Your City:
Your Slack Name:

Homework Details

Module:
Week:

Notes

What did you find easy?
What did you find hard?
What do you still not understand?
Any other notes?

sSalem123 · 2021-06-11T14:15:51Z

I did my new branch and pull request after i finished the work

bonboh

Good effort @sSalem123! I've left some comments and tips for you to read, but you don't need to do anything else 👍

bonboh · 2021-06-22T20:30:12Z

+    ul {
+        display: flex;
+        flex-direction: row;
+        justify-content: space-around;
+        margin-left: 550px;
+        margin-right: 150px;
+    }


Suggested change

ul {

display: flex;

flex-direction: row;

justify-content: space-around;

margin-left: 550px;

margin-right: 150px;

}

.header {

display: flex;

justify-content: space-between;

padding: 1rem;

}

.header img {

height: 2rem;

width: 2rem;

}

ul {

display: flex;

list-style: none;

margin: 0;

}

.header li {

padding: 1rem;

}

Instead of using a specific margin-left to create space between the logo and the links, we can do this instead.

We can use flex to put space between the logo and the links

We can add padding to the list links so that there's space between them

bonboh · 2021-06-22T20:30:49Z

+    .wifi {
+        padding-left: 240px;
+        padding-top: 380px;
+        margin-left: 70px;
+        margin-right: 70px;
+        background-image: url("file:///C:/Users/New%20User/Documents/GitHub/HTML-CSS-Coursework-Week1/HTML-CSS-Coursework-Week2/img/first-background.jpg");
+        display: flex;
+        justify-content: center;
+    }


Suggested change

.wifi {

padding-left: 240px;

padding-top: 380px;

margin-left: 70px;

margin-right: 70px;

background-image: url("file:///C:/Users/New%20User/Documents/GitHub/HTML-CSS-Coursework-Week1/HTML-CSS-Coursework-Week2/img/first-background.jpg");

display: flex;

justify-content: center;

}

We can move this to .karma, because we don't need both the .wifi and .karma sections, just one is enough

bonboh · 2021-06-22T20:32:04Z

+    .little {
+        display: flex;
+        align-content: center;
+        flex-direction: row;


Suggested change

flex-direction: row;

flex-direction is row by default, so we don't need to add this

bonboh · 2021-06-22T20:34:02Z

+    .karma {
+        color: white;
+        display: flex;
+        flex-direction: column;
+        align-items: center;
+        margin-bottom: 120px;
+        font-size: 20px;
+    }


Suggested change

.karma {

color: white;

display: flex;

flex-direction: column;

align-items: center;

margin-bottom: 120px;

font-size: 20px;

}

.karma {

color: white;

display: flex;

flex-direction: column;

align-items: center;

margin-bottom: 120px;

font-size: 20px;

/* moved from .wifi */

height: 40rem;

background-image: url("../img/first-background.jpg");

justify-content: center;

}

We can move these properties from .wifi. We also should use a relative URL instead of putting in file://C:/....

bonboh · 2021-06-22T20:35:28Z

+        width: 130px;
+        height: 50px;


Suggested change

width: 130px;

height: 50px;

padding: 1rem;

Instead of adding a specific width and height, we can add padding. We should also use relative units like rem instead of absolute ones like px

bonboh · 2021-06-22T20:37:53Z

+                <a>meet karma</a>
+                <a>how its works</a>
+                <div id="red"><a>store</a></div>
+                <a>blog</a>
+                <a>help</a>
+                <a>login</a>


Suggested change

<a>meet karma</a>

<a>how its works</a>

<div id="red"><a>store</a></div>

<a>blog</a>

<a>help</a>

<a>login</a>

<li><a>meet karma</a></li>

<li><a>how its works</a></li>

<li id="red"><a>store</a></li>

<li><a>blog</a></li>

<li><a>help</a></li>

<li><a>login</a></li>

These links are within a ul element, so we should put them in li ones

bonboh · 2021-06-22T20:38:33Z

-    <!-- Add your HTML markup here -->
-    <!-- Remember: Use semantic HTML tags like <header>, <main>, <nav>, <footer>, <section> etc -->
-    <!-- All the images you need are in the 'img' folder -->
+        <div class="wifi">


Suggested change

<div class="wifi">

We don't need this .wifi div — the .karma section is enough

bonboh · 2021-06-22T20:39:31Z

-    <!-- All the images you need are in the 'img' folder -->
+        <div class="wifi">
+            <section class="karma">
+                <h1>interducing karma</h1>


Suggested change

<h1>interducing karma</h1>

<h1>Introducing Karma</h1>

We should try to use the same letters as the mockup screenshots, because this is what users see

bonboh · 2021-06-22T20:40:53Z

+        </div>

+        <div class="h3">
+            <h1>Everyone need a little karma</h1>


Suggested change

<h1>Everyone need a little karma</h1>

<h2>Everyone need a little karma</h2>

We should only have one h1, so we can change this to h2 instead

bonboh · 2021-06-22T20:41:23Z

+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 128 128">
+                <path fill="#F05A29"
+                    d="M122 66.463h-1.106v-48.41c0-3.033-2.468-5.5-5.5-5.5H13.521a5.506 5.506 0 00-5.5 5.5v38.126h-2.02a5.506 5.506 0 00-5.5 5.5v51.996c0 3.032 2.467 5.5 5.5 5.5h35.546c3.033 0 5.5-2.468 5.5-5.5V90.627H57.69v10.04h-5.422a1.5 1.5 0 100 3H81.5a1.5 1.5 0 000-3h-5.421v-10.04h23.944v16.562c0 3.032 2.468 5.5 5.5 5.5H122c3.032 0 5.5-2.468 5.5-5.5V71.963c0-3.033-2.468-5.5-5.5-5.5zm-77.953 47.212c0 1.379-1.122 2.5-2.5 2.5H6.001a2.503 2.503 0 01-2.5-2.5v-4.286h40.546v4.286zm0-7.286H3.501v-44.71c0-1.378 1.122-2.5 2.5-2.5l35.546-.001c1.378 0 2.5 1.122 2.5 2.5v44.711zm29.032-5.722h-12.39v-10.04h12.39v10.04zm-26.032-13.04v-9.548h52.976v9.548H47.047zm52.976-15.664v3.116H47.047V61.678c0-3.033-2.467-5.5-5.5-5.5l-30.525.001V18.053c0-1.378 1.122-2.5 2.5-2.5h101.873c1.379 0 2.5 1.122 2.5 2.5v48.41h-12.371a5.507 5.507 0 00-5.501 5.5zm24.477 35.226c0 1.379-1.121 2.5-2.5 2.5h-16.477a2.502 2.502 0 01-2.5-2.5v-4.08H124.5v4.08zm0-7.08h-21.477V71.963c0-1.379 1.121-2.5 2.5-2.5H122c1.379 0 2.5 1.121 2.5 2.5v28.146z" />
+                </svg>


Suggested change

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 128 128">

<path fill="#F05A29"

d="M122 66.463h-1.106v-48.41c0-3.033-2.468-5.5-5.5-5.5H13.521a5.506 5.506 0 00-5.5 5.5v38.126h-2.02a5.506 5.506 0 00-5.5 5.5v51.996c0 3.032 2.467 5.5 5.5 5.5h35.546c3.033 0 5.5-2.468 5.5-5.5V90.627H57.69v10.04h-5.422a1.5 1.5 0 100 3H81.5a1.5 1.5 0 000-3h-5.421v-10.04h23.944v16.562c0 3.032 2.468 5.5 5.5 5.5H122c3.032 0 5.5-2.468 5.5-5.5V71.963c0-3.033-2.468-5.5-5.5-5.5zm-77.953 47.212c0 1.379-1.122 2.5-2.5 2.5H6.001a2.503 2.503 0 01-2.5-2.5v-4.286h40.546v4.286zm0-7.286H3.501v-44.71c0-1.378 1.122-2.5 2.5-2.5l35.546-.001c1.378 0 2.5 1.122 2.5 2.5v44.711zm29.032-5.722h-12.39v-10.04h12.39v10.04zm-26.032-13.04v-9.548h52.976v9.548H47.047zm52.976-15.664v3.116H47.047V61.678c0-3.033-2.467-5.5-5.5-5.5l-30.525.001V18.053c0-1.378 1.122-2.5 2.5-2.5h101.873c1.379 0 2.5 1.122 2.5 2.5v48.41h-12.371a5.507 5.507 0 00-5.501 5.5zm24.477 35.226c0 1.379-1.121 2.5-2.5 2.5h-16.477a2.502 2.502 0 01-2.5-2.5v-4.08H124.5v4.08zm0-7.08h-21.477V71.963c0-1.379 1.121-2.5 2.5-2.5H122c1.379 0 2.5 1.121 2.5 2.5v28.146z" />

</svg>

<img src="img/icon-devices.svg">

We can link to the icon-devices.svg image instead

I finished the project

6e3b68c

bonboh reviewed Jun 22, 2021

View reviewed changes

bonboh closed this Jun 22, 2021

Navigation Menu

Uh oh!

Uh oh!

glasgow-class5/salem-dayfan-html/css-week2 - #222

glasgow-class5/salem-dayfan-html/css-week2#222
sSalem123 wants to merge 1 commit into
CodeYourFuture:masterfrom
sSalem123:glasgow-class5/salem-dayfan

sSalem123 commented Jun 11, 2021

Uh oh!

sSalem123 commented Jun 11, 2021

Uh oh!

bonboh left a comment

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

bonboh Jun 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    ul {
-        display: flex;
-        flex-direction: row;
-        justify-content: space-around;
-        margin-left: 550px;
-        margin-right: 150px;
-    }
+    .header {
+        display: flex;
+        justify-content: space-between;
+        padding: 1rem;
+    }
+    .header img {
+        height: 2rem;
+        width: 2rem;
+    }
+    ul {
+        display: flex;
+        list-style: none;
+        margin: 0;
+    }
+    .header li {
+        padding: 1rem;
+    }

	<h1>Everyone need a little karma</h1>
	<h2>Everyone need a little karma</h2>

Uh oh!

Uh oh!

Conversation

sSalem123 commented Jun 11, 2021

Your Details

Homework Details

Notes

Uh oh!

sSalem123 commented Jun 11, 2021

Uh oh!

bonboh left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants