create new branch for multipage clone #208

SyedArslanHaider · 2024-09-28T12:11:14Z

Learners, PR Template

Self checklist

I have committed my files one by one, on purpose, and for a reason
I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

netlify · 2024-09-28T12:11:30Z

✅ Deploy Preview for cyf-module-html-css ready!

Name	Link
🔨 Latest commit	`de60727`
🔍 Latest deploy log	https://app.netlify.com/sites/cyf-module-html-css/deploys/670a65e11ffd410008d27ec2
😎 Deploy Preview	https://deploy-preview-208--cyf-module-html-css.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

nsi88

Well done. Keep going.
Left some questions and suggestions.
Feel free to disagree. I don't know CSS much better than you, ahah)
Also feel free to ask if not clear. We can have a call as well

nsi88 · 2024-09-30T10:31:54Z

MultiPage-Clone/css/main.css

 /* Dont' forget to link this file to your HTML in the <head> */
+:root {
+ --clr-Dark-beige:#E6DACD;


Really nice that you're using variables. A good practice

nsi88 · 2024-09-30T10:32:24Z

MultiPage-Clone/css/main.css

+    position: fixed; 
+    top: 0; 
+    width: 100%;
+    z-index: 1000; 


Is it clear for you why z-index is needed?

nsi88 · 2024-09-30T10:36:27Z

MultiPage-Clone/css/main.css

+    display: flex;
+    justify-content: center;
+    align-items: center;
+    gap: 10px;


Not critical at all, just a reminder of our lunch conversation) You usually use em or rem for text and px for images. Otherwise if px is used for text, proportions break when you change the font size in the browser.

nsi88 · 2024-09-30T10:41:57Z

MultiPage-Clone/index.html

+        <span>PROJECT MANAGER</span>
+    </div>
+    <ul class="nav-links">
+        <li><a href="#">ABOUT ME</a></li>


It's a good practice to keep meaning in HTML and visualisation in CSS.
Writing "ABOUT ME" in upper case is a part of visualisation to me.
It can be done in CSS. See https://www.w3schools.com/cssref/pr_text_text-transform.php

nsi88 · 2024-09-30T11:55:21Z

MultiPage-Clone/css/main.css

+    width: 16px;
+    height: 16px;
+}
+.logo1 span:nth-child(3){


What do you want to do with the :nth-child selectors?
To have different elements of logo1 to have different font and padding?
Is it easy to write the CSS? Would you understand in a month what you meant?
Can you think of another way to express in CSS what you need?

SyedArslanHaider · 2024-09-30T12:58:32Z

Hi, Sergey Novikov Thanks for your response I'm very happy to see your response ❤️ For Z index : I know it works as layer like if we increase an element z index it's come front and if decrease it go to back Example : I have a paragraph and an image if I decrease the image z index it goes back to the paragraph and paragraph shoes in front of the image Ahh I remember we discussed that PX is not good practice to use most ok I will change it with rm and rem Great I used this property to show text in uper case text-transform: uppercase and it works 🤭 Yes I use nth:child because its parent child relationship and I easily use when needs sometime CSS is not work and I use nth child property that works correctly block property is used to change the behaviour of an element like firstly I use display none then I use display block we can use display inline as well according to requirements

…

On Mon, 30 Sept 2024, 1:57 pm Sergey Novikov, ***@***.***> wrote: ***@***.**** commented on this pull request. Well done. Keep going. Left some questions and suggestions. Feel free to disagree. I don't know CSS much better than you, ahah) Also feel free to ask if not clear. We can have a call as well ------------------------------ In MultiPage-Clone/css/main.css <#208 (comment)> : > /* Dont' forget to link this file to your HTML in the <head> */ +:root { + --clr-Dark-beige:#E6DACD; Really nice that you're using variables. A good practice ------------------------------ In MultiPage-Clone/css/main.css <#208 (comment)> : > + --clr-Dark-beige:#E6DACD; + --clr-Light-beige:#F4ECE6; + --clr-Royal-blue:#0150FD; + --clr-Black:#000; + --clr-White:#FFF; +} +nav { + display: flex; + justify-content: space-between; + align-items: center; + height: 150px; + background: var(--clr-White); + position: fixed; + top: 0; + width: 100%; + z-index: 1000; Is it clear for you why z-index is needed? ------------------------------ In MultiPage-Clone/css/main.css <#208 (comment)> : > +nav { + display: flex; + justify-content: space-between; + align-items: center; + height: 150px; + background: var(--clr-White); + position: fixed; + top: 0; + width: 100%; + z-index: 1000; +} +.logo{ + display: flex; + justify-content: center; + align-items: center; + gap: 10px; Not critical at all, just a reminder of our lunch conversation) You usually use em or rem for text and px for images. Otherwise if px is used for text, proportions break when you change the font size in the browser. ------------------------------ In MultiPage-Clone/index.html <#208 (comment)> : > <body> +<nav> + <div class="logo"> + <div class="logo1"> + <img src="https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcQNgjV6Hxw34EU6zli1YKdNKY45CCCzDtqjKkUsX-uIh3nYTEMMFSL_CMb0Y00h16Hs0uo&usqp=CAU"> + <span>Maya Nelson</span> + <span>/</span> + </div> + + <span>PROJECT MANAGER</span> + </div> + <ul class="nav-links"> + <li><a href="#">ABOUT ME</a></li> It's a good practice to keep meaning in HTML and visualisation in CSS. Writing "ABOUT ME" in upper case is a part of visualisation to me. It can be done in CSS. See https://www.w3schools.com/cssref/pr_text_text-transform.php ------------------------------ In MultiPage-Clone/index.html <#208 (comment)> : > + <div class="logo"> + <div class="logo1"> + <img src="https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcQNgjV6Hxw34EU6zli1YKdNKY45CCCzDtqjKkUsX-uIh3nYTEMMFSL_CMb0Y00h16Hs0uo&usqp=CAU"> + <span>Maya Nelson</span> + <span>/</span> + </div> + + <span>PROJECT MANAGER</span> + </div> + <ul class="nav-links"> + <li><a href="#">ABOUT ME</a></li> + <li><a href="#">RESUME</a></li> + <li><a href="#">PROJECT</a></li> + <li><a href="#">CONTACT</a> + </ul> + <div class="navimage"> What is the block used for? ------------------------------ In MultiPage-Clone/main.js <#208 (comment)> : > \ No newline at end of file +burgerr.addEventListener('click', () => { You want that the link changes the way it looks when clicked, right? Have a look at CSS :active selector https://www.w3schools.com/cssref/sel_active.php. Probably you don't need javascript for that at all. ------------------------------ In MultiPage-Clone/css/main.css <#208 (comment)> : > + justify-content: center; + align-items: center; + gap: 10px; + margin-left: 2rem; +} +.logo1{ + display: flex; + gap: 10px; + justify-content: center; + align-items: center; +} +.logo1 img{ + width: 16px; + height: 16px; +} +.logo1 span:nth-child(3){ What do you want to do with the :nth-child selectors? To have different elements of logo1 to have different font and padding? Is it easy to write the CSS? Would you understand in a month what you meant? Can you think of another way to express in CSS what you need? — Reply to this email directly, view it on GitHub <#208 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHWOUAKPUMZRRGY2AVX655TZZE4EJAVCNFSM6AAAAABPASIOOCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMZXGAZTMOBQGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

nsi88 · 2024-09-30T14:37:09Z

About the z index. You understand it right. You take an element out of the current layer and show it in front or back.
There are cases when it's not the tool though.
For your example for putting an image to the background, usually it's done with background-image property.
In the code here, the nav element doesn't even need to be in front of the main. From what I understood from the template they just go one after the other. In case of the nav I don't think you need z-index at all. Try it out

nsi88 · 2024-09-30T14:40:23Z

Yes I use nth:child because its parent child relationship and I easily use
when needs sometime CSS is not work and I use nth child property that works
correctly

I agree that it works. But it's unreadable to be honest) When you will start working as a programmer you will meet a lot of comments about code readability)
When you look at css you can't say what element the rule aplies to until you go to the template and count the elements.
The usual way to it is to use classes. You use them in other places.
You would have a selector like: .logo .name, .logo .job-title, etc.

nsi88 · 2024-09-30T14:41:36Z

Btw, you can answer to each my comment separately) This way I don't need to search for my comment to remember what we're talking about)

nsi88 · 2024-09-30T14:41:43Z

Keep going. Good job

SyedArslanHaider · 2024-09-30T15:00:17Z

Ok I will try thanks for your quick responses ❤️

…

On Mon, 30 Sept 2024, 4:42 pm Sergey Novikov, ***@***.***> wrote: Keep going. Good job — Reply to this email directly, view it on GitHub <#208 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHWOUAIAUU6XJ5LEC3CCSODZZFPL7AVCNFSM6AAAAABPASIOOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBTGQYDMMJVGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

nsi88 · 2024-10-03T07:02:52Z

MultiPage-Clone/css/resume.css

@@ -0,0 +1,647 @@
+* {


Don't you see any problems with having a separate css file per html page?

nsi88 · 2024-10-03T07:04:30Z

MultiPage-Clone/css/resume.css

+  font-family: Quicksand;
+}
+
+.nav-links li:first-child a {


For resume it should be the second child.
But usually people don't count children in css. They just use an extra class. For example "active" or something like this.

nsi88 · 2024-10-03T07:08:23Z

MultiPage-Clone/index.html

+            <p>Call</p>
+            <span>123-456-7890</span>
+        </div>
+        <div class="second">


What was the difficulty with using a single class instead of 3 separate classes: one, second, third?
You need to be able to add similar html elements without changing css.

Same story for footer-left and footer-right.
Have you tried to use a single class for that?

Class means "one of". For example "one of footer items".
If it doesn't read like this, there is something wrong probably.
"One of footer right" doesn't read really well. There is just one footer right component.

I'd recommend to check for all cases when classes are used this way and to try to change it to us a single class.
If one element should look differently, for example an active navigation link, it gets an extra class "active".

nsi88 · 2024-10-03T07:12:43Z

MultiPage-Clone/main.js


-const OVERFLOW = $('#toggle1');
+burgerr.addEventListener('click', () => {


Right now we just reload a page on click. No javascript is used basically.
Is there a requirement for the task to reload only a part of the page as in the demo?

SyedArslanHaider · 2024-10-03T14:39:05Z

*Hi, * *Sergey Novikov Thanks for your response * I use active class to show the nav link active instead of using the nth-child pseudo-class yes correct I use three different classes my work is done if I use only one class the reason why i use three different classes because when i use media queries for mobile i use display grid on fot-right to position the element but now I change it and use nth-child pseudo-class for positioning grid element also, I use fot-left and fot-right two different classes in footer because in for-left i use flex-direction:column and fot-right i use flex-direction:row I use same code for header and footer in both HTML files because I did't know how it is possible with javascript to write only one header and footer and use it in all pages

…

On Thu, Oct 3, 2024 at 9:20 AM Sergey Novikov ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In MultiPage-Clone/css/resume.css <#208 (comment)> : > @@ -0,0 +1,647 @@ +* { Don't you see any problems with having a separate css file per html page? ------------------------------ In MultiPage-Clone/css/resume.css <#208 (comment)> : > +ul { + margin-right: 2rem; +} + +.nav-links li { + padding: 0 20px; +} + +.nav-links a { + color: var(--clr-Black); + text-decoration: none; + font-size: 15px; + font-family: Quicksand; +} + +.nav-links li:first-child a { For resume it should be the second child. But usually people don't count children in css. They just use an extra class. For example "active" or something like this. ------------------------------ In MultiPage-Clone/index.html <#208 (comment)> : > + <p>I’m a great place for you to tell a story and let your users know a little more about you.</p> + </div> + </div> + </div> +</main> +<footer> + <div class="fot-left"> + <span>© 2035 by Maya Nelson.</span> + <span>Powered and secured by <u>Wix</u></span> + </div> + <div class="fot-right"> + <div class="one"> + <p>Call</p> + <span>123-456-7890</span> + </div> + <div class="second"> What was the difficulty with using a single class instead of 3 separate classes: one, second, third? You need to be able to add similar html elements without changing css. ------------------------------ In MultiPage-Clone/main.js <#208 (comment)> : > -const OVERFLOW = $('#toggle1'); +burgerr.addEventListener('click', () => { Right now we just reload a page on click. No javascript is used basically. Is there a requirement for the task to reload only a part of the page as in the demo <https://www.wix.com/website-template/view/html/2622>? ------------------------------ In MultiPage-Clone/index.html <#208 (comment)> : > + <p>I’m a great place for you to tell a story and let your users know a little more about you.</p> + </div> + </div> + </div> +</main> +<footer> + <div class="fot-left"> + <span>© 2035 by Maya Nelson.</span> + <span>Powered and secured by <u>Wix</u></span> + </div> + <div class="fot-right"> + <div class="one"> + <p>Call</p> + <span>123-456-7890</span> + </div> + <div class="second"> Same story for footer-left and footer-right. Have you tried to use a single class for that? ------------------------------ In MultiPage-Clone/index.html <#208 (comment)> : > + <p>I’m a great place for you to tell a story and let your users know a little more about you.</p> + </div> + </div> + </div> +</main> +<footer> + <div class="fot-left"> + <span>© 2035 by Maya Nelson.</span> + <span>Powered and secured by <u>Wix</u></span> + </div> + <div class="fot-right"> + <div class="one"> + <p>Call</p> + <span>123-456-7890</span> + </div> + <div class="second"> Class means "one of". For example "one of footer items". If it doesn't read like this, there is something wrong probably. "One of footer right" doesn't read really well. There is just one footer right component. ------------------------------ In MultiPage-Clone/index.html <#208 (comment)> : > + <p>I’m a great place for you to tell a story and let your users know a little more about you.</p> + </div> + </div> + </div> +</main> +<footer> + <div class="fot-left"> + <span>© 2035 by Maya Nelson.</span> + <span>Powered and secured by <u>Wix</u></span> + </div> + <div class="fot-right"> + <div class="one"> + <p>Call</p> + <span>123-456-7890</span> + </div> + <div class="second"> I'd recommend to check for all cases when classes are used this way and to try to change it to us a single class. If one element should look differently, for example an active navigation link, it gets an extra class "active". — Reply to this email directly, view it on GitHub <#208 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHWOUAIIVLJTOGTVR25P4PLZZTV5HAVCNFSM6AAAAABPASIOOCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNBUHAYDAMJXGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

nsi88 · 2024-10-04T07:09:27Z

Ok, let's pair program a bit next time we meet. Probably easier this way.

SyedArslanHaider · 2024-10-04T07:30:00Z

Yeah, that's a good idea.

…

On Fri, 4 Oct 2024, 9:09 am Sergey Novikov, ***@***.***> wrote: Ok, let's pair program a bit next time we meet. Probably easier this way. — Reply to this email directly, view it on GitHub <#208 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHWOUALRAATUHJDHXV26O3DZZY5L3AVCNFSM6AAAAABPASIOOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJSHE3TSOJTGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

… sreen 400px

nsi88 · 2024-10-08T07:17:07Z

MultiPage-Clone/css/project.css

@@ -326,6 +334,139 @@ ul {
   }
 }

+@media screen and (min-width: 900px) {


Will complicate a bit the task for you)
Do all the properties below change comparing to other screen sizes?
If not it's better to have them outside of the media query block.
Less code to maintain and to load to browser.

I saw you deleted some pieces of media queries. Well done. Keep going

nsi88

Will comment one thing again because it's important.
For when you make it work. Try to deduplicate the code as much as you can.

The majority of styles repeat across pages. Try to have a single css file. Or one css with common styles and then page specific css files with a minimum of content.
Try to move common styles for all page sizes outside of media queries.

Having the duplication makes you double the work as well.
Also you risk to have different styles on different pages.

SyedArslanHaider · 2024-10-09T07:49:43Z

Thank you for the feedback! I understand the importance of deduplicating the code and will focus on organizing the CSS better, reducing redundancy, and moving common styles out of media queries.I appreciate the advice and will apply it moving forward.

…

On Wed, 9 Oct 2024, 9:31 am Sergey Novikov, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In MultiPage-Clone/css/project.css <#208 (comment)> : > @@ -326,6 +334,139 @@ ul { } } ***@***.*** screen and (min-width: 900px) { I saw you deleted some pieces of media queries. Well done. Keep going — Reply to this email directly, view it on GitHub <#208 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHWOUAIEDUO5UT24EIK4OJLZ2TLWHAVCNFSM6AAAAABPASIOOCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNJWGI4DSNZXGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

…n an element on resume page

create new branch for multipage clone

bb88349

SyedArslanHaider added 2 commits September 29, 2024 13:58

first commit multipage

e0cf203

my second commit

f4f9ec6

nsi88 reviewed Sep 30, 2024

View reviewed changes

SyedArslanHaider added 11 commits September 30, 2024 17:04

my third commit I create first page in web view

b961583

4rth commit add some media queries

db4165f

i added some changes inside media query

3bac1c7

change footer size

6064a87

change media quires

98c99f3

I change some css for footer using media query

701a504

adjusting media quieries

3d5f391

adjusting footer

c64cde0

change some font sizes in main section

ac03464

I created resume.html and resume.css and create mobile design first

a4404de

I add code inside resume.html and resume.css using media query for web

536ff65

nsi88 reviewed Oct 3, 2024

View reviewed changes

Add an active class in nav header and some changes in footer

5516b6f

SyedArslanHaider added 3 commits October 6, 2024 18:57

i add project page for html and css both and code according to mobile…

28bdebc

… sreen 400px

add media quieries according to 1200px screen on project.html

ffdbaf2

fix some padding and margin on media quieries

2ec1d35

nsi88 reviewed Oct 8, 2024

View reviewed changes

SyedArslanHaider added 2 commits October 8, 2024 10:01

delete some extra media quieries inside project.css file

3fa97e4

added contact.html and contact.css

e71e0c2

nsi88 reviewed Oct 9, 2024

View reviewed changes

SyedArslanHaider added 2 commits October 9, 2024 13:23

add 2 buttons onclick code in index.html page and also set position o…

a9759b1

…n an element on resume page

adjust card box-shadow background color

de60727

UmairRiazDev mentioned this pull request Oct 20, 2024

[TECH ED] Code review UmairRiazDev/Coursework-Planner#130

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create new branch for multipage clone #208

create new branch for multipage clone #208

SyedArslanHaider commented Sep 28, 2024

netlify bot commented Sep 28, 2024 •

edited

Loading

nsi88 left a comment

nsi88 Sep 30, 2024

nsi88 Sep 30, 2024

nsi88 Sep 30, 2024

nsi88 Sep 30, 2024

nsi88 Sep 30, 2024

SyedArslanHaider commented Sep 30, 2024 via email

nsi88 commented Sep 30, 2024

nsi88 commented Sep 30, 2024

nsi88 commented Sep 30, 2024

nsi88 commented Sep 30, 2024

SyedArslanHaider commented Sep 30, 2024 via email

nsi88 Oct 3, 2024

nsi88 Oct 3, 2024

nsi88 Oct 3, 2024

nsi88 Oct 3, 2024

nsi88 Oct 3, 2024

nsi88 Oct 3, 2024

nsi88 Oct 3, 2024

SyedArslanHaider commented Oct 3, 2024 via email

nsi88 commented Oct 4, 2024

SyedArslanHaider commented Oct 4, 2024 via email

nsi88 Oct 8, 2024

nsi88 Oct 9, 2024

nsi88 left a comment

SyedArslanHaider commented Oct 9, 2024 via email


		const OVERFLOW = $('#toggle1');
		burgerr.addEventListener('click', () => {

create new branch for multipage clone #208

Are you sure you want to change the base?

create new branch for multipage clone #208

Conversation

SyedArslanHaider commented Sep 28, 2024

Learners, PR Template

Changelist

Questions

netlify bot commented Sep 28, 2024 • edited Loading

✅ Deploy Preview for cyf-module-html-css ready!

nsi88 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SyedArslanHaider commented Sep 30, 2024 via email

nsi88 commented Sep 30, 2024

nsi88 commented Sep 30, 2024

nsi88 commented Sep 30, 2024

nsi88 commented Sep 30, 2024

SyedArslanHaider commented Sep 30, 2024 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SyedArslanHaider commented Oct 3, 2024 via email

nsi88 commented Oct 4, 2024

SyedArslanHaider commented Oct 4, 2024 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nsi88 left a comment

Choose a reason for hiding this comment

SyedArslanHaider commented Oct 9, 2024 via email

netlify bot commented Sep 28, 2024 •

edited

Loading