Skip to content

Github-tracker - efstasia#17

Open
efstasia wants to merge 11 commits into
Technigo:mainfrom
efstasia:main
Open

Github-tracker - efstasia#17
efstasia wants to merge 11 commits into
Technigo:mainfrom
efstasia:main

Conversation

@efstasia

@efstasia efstasia commented Oct 1, 2021

Copy link
Copy Markdown

No description provided.

@katiewu1 katiewu1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice responsive web page! 🤩 Met all the blue level requirements ✅ Your page is very clean and pretty. Your code is also very clean and easy to follow!
I'm a huge fan of comments so it made it easier for me to follow the code 👍 Great job! 👏 🎉

Comment thread README.md
## The problem

Describe how you approached to problem, and what tools and techniques you used to solve it. How did you plan? What technologies did you use? If you had more time, what would be next?
I had a problem with knowing how to start writing the JavaScript. I took a look at last week's project (project-weather-app) and used that as a guide for the API fetch.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Smart approach! 👍

Comment thread code/chart.js
const ctx = document.getElementById("chart").getContext("2d");

//"Draw" the chart here 👇
Chart.defaults.font.size = 16;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice! Looks more easier to change the font size here (I put it inside the datasets... 😅 )

Comment thread code/index.html
</head>
<body>
<header>
<button class="toggleBtn" onclick="myFunction()">Toggle dark mode</button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cool feature 😍

Comment thread code/index.html
<script src="./chart.js"></script>
</body>
</html> No newline at end of file
<section class="grid-container">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe use a more descriptive class name for the section?

Comment thread code/script.js
`;
});
};
profile(); // invoking the profile function

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Really like all the explaining comments, easy to follow! 💯

Comment thread code/style.css
text-align: center;
}
/* variables to style the dark mode, add these in another variable in dark mode and add those to the selectors */
:root {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great idea with root! So much easier to try change colors and so on 😃

Comment thread code/style.css
margin: auto;
}

fieldset {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Like the fieldset, seems quite easy to create (maybe I can try this out for the next project) ⭐

Comment thread code/style.css
}

img {
animation: myAnim 2s ease 0s 1 normal forwards;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Always some kind of animation in your projects, cool! 😉

Comment thread code/style.css

.links:hover {
transition: all 1s;
background: #5f939a;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I've seen that you have used this color a couple of times. Maybe can try to put this color in the root and store it in a variable? 🦊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants