Skip to content

Scot-Class-5/Karma-Webpage #232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sPuoja wants to merge 2 commits into from
Closed

Scot-Class-5/Karma-Webpage #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a348146
Added required html codes
sPuoja Jun 15, 2021
4e39f98
HTML CSS done
sPuoja Jun 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 158 additions & 0 deletions css/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
body {
font-family: 'Roboto', sans-serif;
-webkit-font-smoothing: antialiased;
font-size: 16px;

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
font-size: 16px;

We should never override the browser font size setting, as this means if the user changes their browser setting font size to 24px then this webpage doesn't increase in size. Also, it's good practice to use relative units like % or rem for font-size instead of px so elements change proportionally when the user changes font size.

}

/**
Expand All @@ -16,4 +17,161 @@ body {
* - When using Flexbox, remember the items you want to move around need to be inside a parent container set to 'display: flex'
*/

/* header section */
header{
display: flex;
justify-content: space-evenly;
align-content: center;

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
align-content: center;
align-items: center;

It can be confusing because of the similar words, but align-items is the one we want here to centre the elements vertically

}

header img{
height: 2.5em;
padding: 1rem ;

}

/* Nav bar */


nav ul{
display: flex;
list-style: none;
justify-content: space-evenly;
width: 50rem;
padding: 1rem 0rem;

}
Comment on lines +36 to +43

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
nav ul{
display: flex;
list-style: none;
justify-content: space-evenly;
width: 50rem;
padding: 1rem 0rem;
}
nav ul li {
display: inline-block;
list-style: none;
justify-content: space-evenly;
padding: 1rem;
}

Instead of setting a specific width for the menu links and using flexbox, we can turn the links into inline boxes so they are horizontal and they will have their own natural width


nav ul a{
text-decoration: none;
font-size: 1.5rem;
color:rgb(197, 189, 189);
}

nav ul a:hover{

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
nav ul a:hover{
nav ul a:focus,
nav ul a:hover{

When adding hover, we should also add focus so keyboard users can use the Tab key to jump to this and also get the same style

color:rgb(240, 104, 13);
}

li:first-of-type a{
color: rgb(105, 102, 102);
font-weight: 500;
}
x-special/nautilus-clipboard

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
x-special/nautilus-clipboard

Looks like this was accidentally left in!


/*Main content*/
.hero{
background-position: center;
Comment on lines +62 to +63

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
.hero{
background-position: center;
.hero{
height: 40rem;
background-image: url("../img/first-background.jpg");
background-position: center;

Instead of putting the background image in HTML, we can put it in here and give the element a specific height

background-repeat: no-repeat;
background-size: cover;
position: relative;

}

.hero img{
display: flex;
justify-content: center;
padding-left: 1rem;
width: 102vh;
height: 100%;
}


.hero-text{
color: white;
position: absolute;
top: 30%;
left: 47%;
transform: translate(-50%, -50%);
text-align: center;
}
Comment on lines +79 to +86

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
.hero-text{
color: white;
position: absolute;
top: 30%;
left: 47%;
transform: translate(-50%, -50%);
text-align: center;
}
.hero{
color: white;
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
text-align: center;
}

If we delete the <div class="hero-text"> in the HTML like my other suggestion, we can use flexbox to align the text and button in the centre which is a better way than using position: absolute


.hero-text h1 {
font-size: 2.5rem;
font-weight: 300;
}

.hero-text h2{
font-size: 1.5rem;
font-weight: 300;
}

.hero button{
background-color:rgb(240, 104, 13) ;
padding: 1rem;
border: none;
border-radius: 10px;
}

/* Icon Section */

.icon img{
width: 15rem;
height: 5rem;
}

.icon-container{
display: flex;
justify-content: center;
flex-direction: row;

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
flex-direction: row;

flex-direction is row by default, so we can remove it. We usually remove code that has no effect

text-align: center;

}
.icon-row{
padding: 1rem 2rem;
}

.icon-row p{
width: auto;

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
width: auto;

Here, width is auto by default so we can remove it as it has no effect

font-size: 1.2rem;
color: rgb(46, 45, 45);
}

.icon h2{
font-size: 2rem;
text-align: center;
padding-bottom: 1.5rem;
color: rgb(46, 45, 45);
font-weight: 400;

}

.icon{
width: auto;
margin: 0 auto;
padding-top: 4rem;
box-shadow: 0px 1px 0px rgb(189, 187, 187);

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
box-shadow: 0px 1px 0px rgb(189, 187, 187);
border-bottom: 1px solid rgb(189, 187, 187);

For a single line, we usually use border instead of box-shadow, so we could change it here

}



/* Footer */

footer{
display: flex;
flex-direction: column;
align-items: center;
}

footer section{
display: flex;
}
Comment on lines +155 to +157

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
footer section{
display: flex;
}

The social media icons are already horizontal so we don't need to use flex here


footer img{
margin: 0.5rem;
height: 1.5rem;
padding: 1rem 1rem;

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
padding: 1rem 1rem;
padding: 1rem;

We usually use the shorthand so we can change it to just 1rem

border: 1px solid grey;
border-radius: 30%;

}

footer h3{
font-size: 1.2rem;
font-weight: 400;
}

footer small{
font-size: 1.2rem;
padding: 1rem;
color: grey;
}s
55 changes: 52 additions & 3 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,59 @@
<link rel="shortcut icon" type="image/x-icon" href="favicon.ico">
</head>
<body>
<header>
<img src="img/karma-logo.svg" alt="Logo">
<nav>
<ul>
<li><a href="#">Meet Karma</a></li>
<li><a href="#">How it Works</a></li>
<li><a href="#">Store</a></li>
<li><a href="#">Blog</a></li>
<li><a href="#">Help</a></li>
<li><a href="#">Login</a></li>
</ul>
</nav>
</header>

<main>
<div class="hero">
<img src="img/first-background.jpg">
<div class="hero-text">
Comment on lines +29 to +30

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<img src="img/first-background.jpg">
<div class="hero-text">

We can remove these, like I mentioned in my CSS suggestions

<h1>Introducing Karma</h1>
<h2>Bring WiFi with you, everywhere you go.<h2>

@bonboh bonboh Jun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<h2>Bring WiFi with you, everywhere you go.<h2>
<p>Bring WiFi with you, everywhere you go.<p>

This doesn't seem like a heading to me — there is already a heading above it — so we could change this to a p

<button>Learn More</button>
</div>
</div>
<div class="icon">
<h2>Everyone needs a little Karma.</h2>
<div class="icon-container">
<div class="icon-row">
<img src="img/icon-devices.svg" alt="All devices">
<p>Internet for all devices</p>
</div>
<div class="icon-row">
<img src="img/icon-coffee.svg" alt="Coffee Mug">
<p>Boost your Productivity</p>
</div>
<div class="icon-row">
<img src="img/icon-refill.svg" alt="Gas Refill Tank">
<p>Pay as You Go</p>
</div>
</div>
</div>
</main>
<footer>
<h3>Join us on</h3>
<section>
<img src="img/twitter-icon.svg" alt="twitter logo">
<img src="img/facebook-icon.svg" alt="facebook logo">
<img src="img/instagram-icon.svg" alt="instagram logo">
</section>

<!-- 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 -->
<small>&copy; Karma Mobility. Inc.</small>

</footer>


</body>
</html>