Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 14 additions & 2 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link rel="stylesheet" href="style.css">
<!--
Why did you choose to rely on DOMContentLoaded in this script and defer the axios dependency?
After some googling, it appears this will work reliably in chrome (that is, index.js runs after axios is loaded)
but I'm curious how you wound up with this!
-->
<script src="index.js"></script>
<script src="https://unpkg.com/axios/dist/axios.min.js" defer></script>
<title>Damien Yule</title>
Expand All @@ -27,7 +32,7 @@
</ul>
</div>
</div>

<!-- Write your javascript in a javascript file!!!! None of these semi-colon separated mulitline onclick attributes! -->
<button class="menu scroll-btn"
onclick="this.classList.toggle('opened');this.setAttribute('aria-expanded', this.classList.contains('opened'))"
aria-label="Main Menu">
Expand All @@ -54,6 +59,7 @@

<section>
<div class="hero">
<!-- Why not use h1 - h6 tags here? Seems like it might simplify both your html and css a bit, and tells me more about what content lives in the tags! -->
<div class="hello-text">
<p>Hi, my name is</p>
</div>
Expand Down Expand Up @@ -92,6 +98,7 @@
<div class="about-right card">
<div class="card__face card__face--front">
<h3>About Me</h3>
<!-- Cool! I'd love to hear more about your ml and data science interests sometime : ) -->
<p>My main interests are in machine learning and artificial intelligence, but I also have strong
interest in
data science.</p>
Expand Down Expand Up @@ -136,6 +143,9 @@ <h3>Back End</h3>
</div>
</section>
<section class="projects">
<!-- Instead of your projects section being laid out as 3 separate grids, your code might be simpler and more maintainable if
you used a single grid!
-->
<h2>Projects</h2>

<div class="PMA">
Expand Down Expand Up @@ -292,6 +302,8 @@ <h2>Get in Touch</h2>
</div>
<div><button class="hero-button say-hello">Say Hello</button></div>
<div class="icon-para">
<!-- Hmmm... what's up with this tag? I don't believe side-bar is part of the HTML spec! -->
<!-- I'm also curious: why did you choose to nest this inside section.contact-section? -->
<side-bar>
<ul class="sidebar">
<li><a href="https://github.com/DamienYule" target="_blank"><svg role="img" viewBox="0 0 24 24"
Expand Down Expand Up @@ -338,4 +350,4 @@ <h2>Get in Touch</h2>
<img src="https://via.placeholder.com/50" alt="codepen">
<img src="https://via.placeholder.com/50" alt="github">
<img src="https://via.placeholder.com/50" alt="instagram"> -->
<!-- <svg role="img" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" version="1.1" id="Layer_1" x="0px" y="0px" viewBox="0 0 101.15 122.88" ><g><path d="M18.03,27.19c8.26,2.76,19.76,4.46,32.53,4.46c12.77,0,24.27-1.71,32.53-4.46c7.25-2.42,11.74-5.35,11.74-8.22 c0-2.87-4.48-5.8-11.74-8.22C74.83,8,63.33,6.29,50.56,6.29c-12.77,0-24.27,1.71-32.53,4.46C2.65,15.89,2.22,21.91,18.03,27.19 L18.03,27.19z M94.84,85.59c-2.58,1.77-5.87,3.32-9.76,4.62c-8.9,2.97-21.11,4.81-34.52,4.81c-13.41,0-25.62-1.84-34.52-4.81 c-3.84-1.28-7.11-2.82-9.68-4.55v18.85c0.57,2.67,4.92,5.37,11.67,7.62c8.26,2.76,19.76,4.46,32.53,4.46s24.27-1.71,32.53-4.46 c5.01-1.67,8.69-3.59,10.5-5.55c1.49-1.62,1.25-2.69,1.25-4.64V85.59L94.84,85.59z M0,18.97C0,13.1,6.13,8.12,16.04,4.81 C24.94,1.84,37.15,0,50.56,0c13.41,0,25.62,1.84,34.52,4.81c9.02,3.01,14.91,7.41,15.89,12.61c0.12,0.33,0.18,0.69,0.18,1.06v86.74 c0,6.01-11.49,11.33-16.07,12.86c-8.9,2.97-21.11,4.81-34.52,4.81c-13.41,0-25.62-1.84-34.52-4.81 c-4.69-1.57-15.97-6.71-15.97-12.86c0-0.72,0-1.32,0-2.01C0.07,75.12,0,47.04,0,18.97L0,18.97z M6.36,76.64 c0.57,2.67,4.92,5.37,11.67,7.62c8.26,2.76,19.76,4.46,32.53,4.46s24.27-1.71,32.53-4.46c7.25-2.42,11.74-5.35,11.74-8.22h0.03 V57.73c-2.58,1.77-5.89,3.32-9.78,4.62c-8.9,2.97-21.11,4.81-34.52,4.81c-13.41,0-25.62-1.84-34.52-4.81 c-3.84-1.28-7.11-2.82-9.68-4.55V76.64L6.36,76.64z M6.36,48.78c0.57,2.67,4.92,5.37,11.67,7.62c8.26,2.76,19.76,4.46,32.53,4.46 s24.27-1.71,32.53-4.46c7.25-2.42,11.74-5.35,11.74-8.22h0.03V28.52c-2.58,1.77-5.89,3.32-9.78,4.62 c-8.9,2.97-21.11,4.81-34.52,4.81c-13.41,0-25.62-1.84-34.52-4.81c-3.84-1.28-7.11-2.82-9.68-4.55V48.78L6.36,48.78z"/></g></svg> -->
<!-- <svg role="img" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" version="1.1" id="Layer_1" x="0px" y="0px" viewBox="0 0 101.15 122.88" ><g><path d="M18.03,27.19c8.26,2.76,19.76,4.46,32.53,4.46c12.77,0,24.27-1.71,32.53-4.46c7.25-2.42,11.74-5.35,11.74-8.22 c0-2.87-4.48-5.8-11.74-8.22C74.83,8,63.33,6.29,50.56,6.29c-12.77,0-24.27,1.71-32.53,4.46C2.65,15.89,2.22,21.91,18.03,27.19 L18.03,27.19z M94.84,85.59c-2.58,1.77-5.87,3.32-9.76,4.62c-8.9,2.97-21.11,4.81-34.52,4.81c-13.41,0-25.62-1.84-34.52-4.81 c-3.84-1.28-7.11-2.82-9.68-4.55v18.85c0.57,2.67,4.92,5.37,11.67,7.62c8.26,2.76,19.76,4.46,32.53,4.46s24.27-1.71,32.53-4.46 c5.01-1.67,8.69-3.59,10.5-5.55c1.49-1.62,1.25-2.69,1.25-4.64V85.59L94.84,85.59z M0,18.97C0,13.1,6.13,8.12,16.04,4.81 C24.94,1.84,37.15,0,50.56,0c13.41,0,25.62,1.84,34.52,4.81c9.02,3.01,14.91,7.41,15.89,12.61c0.12,0.33,0.18,0.69,0.18,1.06v86.74 c0,6.01-11.49,11.33-16.07,12.86c-8.9,2.97-21.11,4.81-34.52,4.81c-13.41,0-25.62-1.84-34.52-4.81 c-4.69-1.57-15.97-6.71-15.97-12.86c0-0.72,0-1.32,0-2.01C0.07,75.12,0,47.04,0,18.97L0,18.97z M6.36,76.64 c0.57,2.67,4.92,5.37,11.67,7.62c8.26,2.76,19.76,4.46,32.53,4.46s24.27-1.71,32.53-4.46c7.25-2.42,11.74-5.35,11.74-8.22h0.03 V57.73c-2.58,1.77-5.89,3.32-9.78,4.62c-8.9,2.97-21.11,4.81-34.52,4.81c-13.41,0-25.62-1.84-34.52-4.81 c-3.84-1.28-7.11-2.82-9.68-4.55V76.64L6.36,76.64z M6.36,48.78c0.57,2.67,4.92,5.37,11.67,7.62c8.26,2.76,19.76,4.46,32.53,4.46 s24.27-1.71,32.53-4.46c7.25-2.42,11.74-5.35,11.74-8.22h0.03V28.52c-2.58,1.77-5.89,3.32-9.78,4.62 c-8.9,2.97-21.11,4.81-34.52,4.81c-13.41,0-25.62-1.84-34.52-4.81c-3.84-1.28-7.11-2.82-9.68-4.55V48.78L6.36,48.78z"/></g></svg> -->
7 changes: 7 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Have you compared/contrast visibility to display: none?
// For some of your use cases here I think display: none may be more appropriate.

document.addEventListener('DOMContentLoaded', () => {
document.addEventListener('scroll', () => {
// console.log("scrolling")
Expand All @@ -20,6 +23,7 @@ document.addEventListener('DOMContentLoaded', () => {
const punchButton = document.querySelector(".punchline-button")
const clear = document.querySelector(".clear")
let result = await axios.get("https://official-joke-api.appspot.com/jokes/programming/random")
// template literal isn't necessary! result.data[0].setup is a string!!
div.innerHTML = `${result.data[0].setup}`
div.style.visibility = "visible"
punchButton.style.visibility = "visible"
Expand All @@ -33,6 +37,7 @@ document.addEventListener('DOMContentLoaded', () => {
clear.style.visibility = "hidden"
})
}
// Could you add the wiping the div's innerHTML to getJoke and just pass getJoke instead of creating a new callback here?
button.addEventListener('click', (event) => {
div.innerHTML = ''
getJoke()
Expand All @@ -42,6 +47,7 @@ document.addEventListener('DOMContentLoaded', () => {
const navBtn = document.querySelector('.scroll-btn')
const scollList = document.querySelector('.scroll-nav-list')
navBtn.addEventListener('click', (event) => {
// remove the console log
console.log("hello")
if(scollList.style.visibility === "visible"){
scollList.style.visibility = "hidden"
Expand All @@ -50,6 +56,7 @@ document.addEventListener('DOMContentLoaded', () => {
scollList.style.visibility = "visible"
navBtn.style.zIndex = "7"
}
// Use classList.toggle to shorten this
if (scollList.classList.contains("showNav")) {
scollList.classList.remove("showNav");
} else {
Expand Down
13 changes: 12 additions & 1 deletion style.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
/* #bae67e */
/*
Looks like you're using BEM notation for the css relating to the flippable cards but not elsewhere...
I'd recommend using it consistently throughout or nowhere! (Or, if that css comes from some specific source, maybe you could house it in a separate css file.)
*/

body {
background-color: #0a192f;
Expand Down Expand Up @@ -40,9 +44,10 @@ h3:hover {
display: grid;
grid-template-columns: 1fr 1fr;
}

/* Nice application of flexbox*/
.nav-list {
display: flex;
/* is align-content ever necessary? I think the navbar gets hidden before the screen is small enough for this to be relevant. */
align-content: space-around;
justify-content: flex-end;
color: #5ccfe6;
Expand Down Expand Up @@ -285,6 +290,7 @@ section {
margin-top: 0px;
font-family: monaco;
}
/* Choose kebab-case (my-name) or camelCase (myName) not both! */
.my-Name p {
font-size: 10vw;
color: #ccd6f6;
Expand Down Expand Up @@ -503,6 +509,10 @@ section {
background-color: #1d2433;
border-radius: 10px;
}
/*
This is a fine use-case for grid, although flexbox might be even more appropriate for a 1-d layout.
Could you have used grid to layout all the card content including the icons?
*/
.skill-list {
display: grid;
grid-template-columns: 1fr 1fr;
Expand Down Expand Up @@ -575,6 +585,7 @@ section {
}

/* ----------------------Skill Icon----------------------------- */
/* This might be content that is more easily laid out with grid! */
.react-icon {
position: absolute;
margin-top: 5%;
Expand Down