Skip to content

Instantly share code, notes, and snippets.

@ross-u
Created July 23, 2020 11:00
Show Gist options
  • Save ross-u/ac00a39b01c38d623039b4c01084cb14 to your computer and use it in GitHub Desktop.
Save ross-u/ac00a39b01c38d623039b4c01084cb14 to your computer and use it in GitHub Desktop.

Jean Baptiste

https://github.com/OnizukaJS/Fit-For-Champions

README

  • Polish the readme using the markdown syntax for the Headlines
  • Add the link of the deployed verision to the README.

HTML

  • When importing fonts from google fonts, select all the fonts and then generate only one link. With multiple links, page has to do several requests for the assests which slows down the initial load.
  • HTML has a lot of div elements. Try using more semantic elements like section and article
  • img tags should have alt attribute. This is the minimum we should put when it comes to following best practices of accessibility for screen readers.
  • some class names are written in camelCase and some in kebab-case syntax. Prefer using kebab-case syntax when naming id or class attributes in HTML.
  • HTML code was well commented. ✅

CSS

  • Avoid using !important for css.

JS

  • JS variables were well named, good job ✅
  • faq.js file is left with commented code, not in use. Remove unused files.
  • Try using array methods for iteration more often instead of for loop. for loops are not bad, but a lot of for loops make the code large and hard to read.
  • I see you were using comments in your js code, good job. Try to make a habit of commenting your code well.
  • Use const instead of let. We use let only for values that we are going to change. When storing objects or arrays we store them in const, as objects are mutable. Example:
// js/hamburger-menu.js

let menuResp = document.getElementById('menuResp');
//	/js/main.js

let elmt = data[i];

let date = new Date(`${elmt.date}`);

Commits

  • Project didn't have many commits. Try making commits more often, and always include a clear commit message with the description of changes up to 50 characters long.


Sergio Centellas

https://github.com/Noiretit/DATanime

README

  • Readme looks good overall.
  • Consider removing the part Wireframes, or adding the wireframes images if you have them.
  • Add the link of the deployed verision of the project to the README.

HTML

  • HTML has a lot of div elements. Try using more semantic elements like section and article
  • img tags are following the best practice and have alt attribute, good job.
  • some class names are written in camelCase and some in kebab-case syntax. Prefer using kebab-case syntax when naming id or class attributes in HTML.
  • HTML code was well commented. ✅

CSS

  • Avoid using !important for css rules.
  • All styles were written in one file. In the future try making your css more modular, having one main file for base styles and splitting the styles for different pages in other files.
  • In the future remember to use the CSS reset boilerplate to clear the browser default styles. If not you may end up having your app looking different between different browsers.
  • Move your font files from css folder to a separate folder to have the file structure more organized.

JS

  • JS variables were well named, good job ✅
  • You have several files left with the commented code not in use. Try removing those comments.
  • Try writting more comments in your js code. This will serve to help you or others navigate and read your code easier. When writting comments try to make them descriptive and concise.
  • Use const instead of let. We use let only for values that we are going to change. When storing objects or arrays we store them in const, as objects are mutable. Example:
// scripts/AnimeListApi.js

let randomAnimeId = Math.round(Math.floor((Math.random() * 400)));
  • The functions created to call to the API could be moved to one file and created as methods of a class, for example:
scripts/AnimeAPIService.js
class AnimeAPI {
  constructor () {
		this.BASE_URL = "https://dat-anime-api.herokuapp.com";
  }
  
  getDetailedAnimeInfo = () => { /* ... */}
  
  getRandomDetailedAnime =() => { /* ... */}
  
  getAnimes() => {/* ... */}
}

Commits



Alejandro Colorado

https://github.com/alejandrocolorado/summer-in-mars

README

  • Readme looks good, but still needs a bit of update:

  • Part with links is not completed, try updating it. Add the link of the deployed verision here so that live version can be accessed.

  • Part Data Structure can be created using VSCode extension, to make it look cleaner. Consider using this VSCode extension (or similar) and pasting it in the code block for example:

    📦project-name
     ┣ 📜README.md
     ┣ 📜index.js
     ┣ 📦 scripts
     ┗ 📦 images
    

HTML

  • HTML has a lot of div elements. Try using more semantic elements like section and article.

  • img tags should have alt attribute. This is the minimum we should put when it comes to following best practices of accessibility for screen readers.

  • class and id names are well named in kebab-case syntax, good job. ✅

  • HTML code could use few comments to describe specific sections of code.

  • in the place of importing fonts inside of the <style></style> tag with the @import, use the <link> tag directly.

    <style>
      @import url("https://fonts.googleapis.com/css2?family=Orbitron:wght@400;700;900&family=Raleway:wght@200;300;400;700&display=swap");
    </style>

CSS

  • Great job with using pseudo selectors. ✅

  • All styles were written in one file. In the future when you will have multiple pages try making your css modular, having one main file for base styles and splitting the styles for different pages in other files. Not a problem at the moment as you have only one page.

  • If you were using materiallize as the styling library, you should add it on your readme. As well if you used any other libraries, it is good to mention it on your README.

  • Try updating the indentation in the css to have it consistent accros the file.

      .login-form {
    height: 400px;
    padding: 0 3rem;
    
      }
  • Use "" quotes in the url of the background-image property.

    .faq-container {
      background-image: url(./../images/mars_nasa.jpg);
      /* ... */
    }

JS

  • JS variables were well named, good job ✅
  • Try writting more comments in your js code. This will serve to help you or others navigate and read your code easier. When writting comments try to make them descriptive and concise.
  • Use const instead of let. We use let only for values that we are going to change. When storing objects or arrays we store them in const, as objects are mutable. Example:
// scripts/AnimeListApi.js

let randomAnimeId = Math.round(Math.floor((Math.random() * 400)));

Commits



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