Skip to content

Ready for deploy#69

Open
eyahya-khan wants to merge 10 commits intoTechnigo:mainfrom
eyahya-khan:main
Open

Ready for deploy#69
eyahya-khan wants to merge 10 commits intoTechnigo:mainfrom
eyahya-khan:main

Conversation

@eyahya-khan
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@isabellwastfelt isabellwastfelt left a comment

Choose a reason for hiding this comment

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

Really nice GitHub tracker!
You added some cool feturs that I really liked. Like the search field.
Your code is easy to read and I understad each part of it.

Check out my tip about where to but the scret.js file in html.

Overall really good job! ⭐

</div>

<script src="./chart.js"></script>
<script src="./secret.js"></script>
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 got a tip to put secret.js above chart.js and script.js so it loads before and therefore get the information faster.


const numberCommit = data.length

//added function so the DOM do not self close tags
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 that you mange to add so much information from github

<header class="header">
<h1 class="header-text">GitHub Tracker</h1>
<div class="search-container">
<form id="search-form">
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 nice with the search function!

animation-delay: -0.5s;
}

@keyframes lds-ripple {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Learned something knew here



/*Loading icon*/
.lds-ripple {
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 this one!

/* desktop */
@media (min-width: 992px) {


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 would add some size to the chart here because it is kinda big in desktop size.

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