-
Notifications
You must be signed in to change notification settings - Fork 30
Jorge Sahagún #22
base: master
Are you sure you want to change the base?
Jorge Sahagún #22
Conversation
<body> | ||
<h1>Hello world!</h1> | ||
<script src="./dist/bundle.js"></script> | ||
<script src="./dist/bundle.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is recommended to load scripts at the bottom of the document, so that users can see the site instead of a blank screen while it loads
this.dotColored = DOT_COLORED; | ||
this.dotUncolored = DOT_UNCOLORED; | ||
this.getSiteElements = new Promise( | ||
(resolve) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is starting to look nested, probably you can refactor this to look cleaner
(resolve) => { | ||
window.onload = function () { | ||
const article = { | ||
title: document.getElementById("blog_title"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just as a recommendation, when selecting nodes with js is better to do it with selectors that are not related with styles and use a prefix, ie: .js-my-class-selector
width: 100px; | ||
} | ||
|
||
// ! BLOG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will get large really soon, just like js files you can import Sass files and separate each section in a file
// MEDIA QUERIES | ||
|
||
// TABLET LANDSCAPE | ||
@media screen and (max-width:1024px){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend having media queries along their base rules for easier reading
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The site looks and works great, good job!
What's your progress on the project?
Where should the reviewer start?
Checklist
console.logs