Leila Farsani-Karma-Clone-Week2 #517

leilafarsani · 2022-10-28T13:56:23Z

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: Leila Farsani
Your City: London
Your Slack Name: Leila Farsani

Homework Details

Module: HTML-CSS-Module-Project
Week: 2

Notes

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

jonnywyatt

Really good work - it looks like the design, and you've used flexbox and semantic HTML effectively!

jonnywyatt · 2022-10-29T20:53:55Z

css/style.css

@@ -1,19 +1,189 @@
+:root{
+    --grey-dark: #363434;


This is a really good use of CSS variables! Did you learn about them in the lesson?

jonnywyatt · 2022-10-29T21:00:31Z

index.html

+    </nav>
+ </header> 
+ <main> 
+    <section class="p1">


Really good clean markup structure, well done. You've used header, main, footer at the top level. <section /> is a good choice for the containers below that (rather than article, because that's for content that could be shared / syndicated with other sites).

jonnywyatt · 2022-10-29T21:01:59Z

index.html

+            Everyone needs alitte Karma.
+        </h2>
+        <div class="features">
+            <div class="p2">


Nearly all your CSS classnames are really clear and easy to understand. Maybe worth changing p1 and p2 as well so when you see them in the CSS you'll remember what they're for.

jonnywyatt · 2022-10-29T21:05:55Z

css/style.css

+    display: flex;
+    flex-direction: column;
+    align-items: center;
+    margin-top: 4rem;


This is why you also needed a negative -60px margin on main, above.
Simpler to not style all sections, use a class selector instead and just target the sections you need to add this margin to, rather than all sections.

jonnywyatt · 2022-10-29T21:07:26Z

css/style.css

+}
+.p1 {
+    background-image: url(/img/first-background.jpg);
+    background-size: cover;


jonnywyatt · 2022-10-29T21:09:54Z

index.html

+ <main> 
+    <section class="p1">
+        <h1>Introducing Karma</h1>
+        <h4>Bring WiFi with you, everywhere you go.</h4>


With the heading structure in a page, it's not valid to jump several levels like this from h1 to h4, so best as either a h2 or a div

jonnywyatt · 2022-10-29T21:11:05Z

index.html

+    <section class="p1">
+        <h1>Introducing Karma</h1>
+        <h4>Bring WiFi with you, everywhere you go.</h4>
+        <button><a href="#">Learn More</a></button>


A link can't be wrapped in a button. This is best as just a <a />, then style that as a button.

jonnywyatt · 2022-10-29T21:11:45Z

index.html

+        <div class="features">
+            <div class="p2">
+              <img  src="img/icon-devices.svg" alt="">
+              <h5>Internet for all devices </h5>


These are fine as <div />, they're not really subheadings as there's not content beneath them.

jonnywyatt · 2022-10-29T21:12:31Z

index.html

+        </h2>
+        <div class="features">
+            <div class="p2">
+              <img  src="img/icon-devices.svg" alt="">


I think this could maybe use an alt, but interesting discussion to have

jonnywyatt · 2022-10-29T21:13:24Z

index.html

+    </section>
+ </main> 
+ <footer>
+   <span><h2>Join us on</h2></span>


No need to wrap a block element like h2 in a span, it will have no effect.

jonnywyatt · 2022-10-29T21:27:57Z

index.html

+        </div>
+    </section>
+ </main> 
+ <footer>


Good use of a semantic tag!

jonnywyatt · 2022-11-06T14:27:52Z

css/style.css

- }
+  display: flex;
+  flex-direction: column;
+  margin-top: -80px;


The layout will be simpler to develop if you remove this line, and also remove the margin-top on the section selector.
Remember, it's best not to style using element selectors, as it will affect every one of them and then you might have to add more CSS to work against those styles, as you have here.

jonnywyatt · 2022-11-06T14:29:24Z

css/style.css

+.part3 {
+  display: grid;
+  grid-template-rows: auto;
+  grid-template-columns: 2fr 3fr;


great! Good use of grid here

jonnywyatt · 2022-11-06T14:33:02Z

css/style.css

+}
+
+.p3-img {
+  background-color: #363434;


Try to use your colour variables everywhere

jonnywyatt · 2022-11-06T14:34:21Z

css/style.css

+  display: flex;
+  flex-direction: column;
+  align-items: center;
+  gap: 2rem;


Great! This spaces out the two elements in the flex container

jonnywyatt · 2022-11-06T14:37:17Z

css/style.css

+.main2 {
+  display: flex;
+  flex-direction: row;
+  flex-wrap: wrap;


I don't think this line will help; you don't want the second section to wrap to the next line

jonnywyatt · 2022-11-06T14:46:46Z

css/style.css

+  display: flex;
+  gap: 10px;
+  list-style: none;
+}


ul has some left margin and padding by default, you could reset them to zero here to make it look closer to the design.

jonnywyatt · 2022-11-06T14:48:27Z

index.html

+        <h1>Order your Karma wifi device today!</h1>
+
+        <form action="#">
+            <div class="fieldset">


Calling this class fieldset is a bit confusing as the div isn't a fieldset. The fieldset tag is used to group several checkboxes or radio buttons together.

jonnywyatt · 2022-11-06T14:49:04Z

index.html

+                </div>
+            </div>
+
+            <div class="radio-box">


This should be a fieldset tag instead of a div, because it's a group of related form controls (radio buttons)

jonnywyatt · 2022-11-06T14:50:03Z

index.html

+    </section>
+
+    <section class="section2-img">
+        <img src="level-2/store-image_by-andrew-neel-unsplash.jpg" alt="">


Maybe this is a decorative image, like the one in the new section on index.html, so you could use CSS background-image to add it

jonnywyatt · 2022-11-06T14:52:30Z

css/style.css

  font-style: italic;
 }
+
+.main2 {


In the design, the two sections under the main tag share the width 50% each. How can you do that here? Maybe grid would be simpler, to do that?

leilafarsani added 8 commits October 27, 2022 13:46

Update index.html

5f687f0

P1

3f66a36

p2

52b1d60

p3

72db205

css1

7d4a0e8

more chnges

07eb3ed

img /

3b04d89

looking better

912dcdf

jonnywyatt reviewed Oct 29, 2022

View reviewed changes

index.html

</div>

</section>

</main>

<footer>

Copy link

jonnywyatt Oct 29, 2022

Choose a reason for hiding this comment

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

Good use of a semantic tag!

leilafarsani added 3 commits November 2, 2022 22:58

Section Added

ddbbba3

karma-store

26561ea

image fixed and pages are connected

5344bd7

jonnywyatt reviewed Nov 6, 2022

View reviewed changes

leilafarsani closed this by deleting the head repository Apr 24, 2024

Uh oh!

Leila Farsani-Karma-Clone-Week2 #517

Leila Farsani-Karma-Clone-Week2 #517

Uh oh!

Conversation

leilafarsani commented Oct 28, 2022

Your Details

Homework Details

Notes

Uh oh!

jonnywyatt 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!

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!

jonnywyatt Nov 6, 2022 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonnywyatt Nov 6, 2022 •

edited

Loading