diff --git a/index.html b/index.html index ab5ba64..461812e 100644 --- a/index.html +++ b/index.html @@ -1,4 +1,5 @@ + @@ -23,6 +24,7 @@
diff --git a/index.js b/index.js index 47214a4..53c37c0 100644 --- a/index.js +++ b/index.js @@ -1,5 +1,6 @@ const splash = document.querySelector('.splash') +// Is it necessary to use this event listener since you used defer? document.addEventListener('DOMContentLoaded', (e) => { setTimeout(() => { splash.classList.add('display-none') @@ -14,6 +15,8 @@ navToggle.addEventListener('click', () => { document.body.classList.toggle('nav-open') }) +// Could you think of a way to accomplish this with a single event +// listener rather than an event listener for each nav link? navLinks.forEach(link => { link.addEventListener('click', () => { document.body.classList.remove('nav-open') diff --git a/style.css b/style.css index baf8956..f33345d 100644 --- a/style.css +++ b/style.css @@ -5,7 +5,7 @@ } /* Custom Properties, update these for my own design */ - +/*Nice use of CSS 'variables'*/ :root { --ff-primary: "Source Sans Pro", sans-serif; --ff-secondary: "Source Code Pro", monospace; @@ -201,7 +201,9 @@ header { transform: translateX(100%); transition: transform 250ms cubic-bezier(.5, 0, .5, 1); } - +/* +Love this use of flexbox - the nav list is quite nicely laid out. +*/ .nav__list { list-style: none; display: flex; @@ -390,7 +392,7 @@ header { @media (min-width: 600px) { .about-me { - display: grid; + display: grid; /*Love this grid on layout on larger screens!!!*/ grid-template-columns: 1fr 200px; grid-template-areas: "title img" @@ -413,6 +415,7 @@ header { padding-right: calc(200px + 4em); } + /*As I resize the page, the aspect ratio of the image changes. How could you fix this? Is the height property necessary here?*/ .about-me__img { grid-area: img; position: relative; @@ -472,6 +475,14 @@ text-align: center; /* Adjust this for for mobile */ +/* +Love to see you've got a start on changing the projects layout for mobile! +max-width: 300px is quite small - you might want to adjust that breakpoint! + +The paragraphs in your skills section look quite nice on mobile - maybe you could use their styles as a guide for +How to update this media query! +*/ + /* @media (max-width: 300px) { .portfolio__item { width: 10px; @@ -525,4 +536,4 @@ text-align: center; .social-list__link { padding: .5em; -} \ No newline at end of file +}