@l3laze
@JEWebDev
Submitted
Sometimes when i deploy a website on github pages the images don't load even if the path is correct. How can i fix it? (same case with font).
Is there something i can improve? (code structure, bad practices, etc).
Any tip for breakpoints and media queries?
@l3laze
Posted
Along with some of the relative paths needing a fix, there's a typo in the favicon which will cause it to fail to load. - <img src="assets//images/icon-star.svg" alt="star icon" /> - two / in a row in the path, which will be interpreted as "assets/". The dev console (Ctrl/Cmd + Shift + i or F12) should mention this if you load the page in it.
I don't know if any self-closing elements still require the trailing /> nowadays, and I don't know if they ever were required on some of these elements. Self-closing are when the tag and all it's data are contained entirely between the opening and closing brackets <...>.
The CSS and JS file are fine in the root folder alongside the HTML file in small projects. If you're more comfortable having them split off though, that won't hurt anybody.
An h1 tag is required for accessibility purposes. If you scroll to the bottom of your solution on FM it should have an element showing an accessibility report that mentions this. There's also a HTML report if you hit the button to switch between the two tabs.
While div does work great in so many situations, HTML is more readable in general for developers with more use of semantic markup. Like heading, paragraph, button, etc, along with class names which can help identify the content and the containing element at once.
Media queries only apply when the rule is met, so currently it's only effective on screens that are 320-800px wide (>= 320 and <= 800). Since the media query seems to be mobile-only it would be best to make it only run on either 799 or 800px or less, like @media screen and (width < 800px) for 799, or <= 800 for 800. This way it will still take effect on screens less than 320px wide, which must be tiny older smartphones and flip phones because it's pretty small.
That JS is beautiful; nice work! Minimal code to accomplish the goal and very easily understandable. Could use some whitespace for formatting between const declaration and loop and then between the two functions, but that's a nitpick (doesn't really matter, more personal preference than industry standards).
A lint tool should be able to automate checking for many of the above issues. In an IDE it may be built-in to the app, or available as an extension, and there are of course a ton of tools for this which run in NodeJS and many other languages. I'm using VSCode and the HTML linter seems to be included, but not CSS or JS. CSS I'm trying Stylelint currently, but I'd be just as willing to try the other popular options. For JS I have used StandardJS for years now, but ESLint is another great option - so much so that Standard uses it behind the scenes, lol.
Marked as helpful
@RolandVels
Submitted
I would like to know is it even correct ? Personally for me look similar as example picture give but not sure if code is correct
@l3laze
Posted
An excellent first shot for a beginner :) Did you use something to help build it, or is this all by hand? Curious because of the CSS in particular - seems like there are both unused and advanced selectors in the top section, and then basic selectors which are all used in the bottom.
If you haven't already, make sure to scroll near the bottom of the page on your FM solution and look for the automatically generated accessibility and HTML reports as they can find things that many humans may miss.
The only issue I can see with the HTML seem like nitpicks:
The outer div is indented one extra space. Easily fixed, and could be automated by a code formatter or warned about by a lint tool.
class=main-border seems like a bad identifier for the main container. A main element, or class of main or main-container, may be better.
CSS (ignoring top half, as it seems generated):
Designs on FM seem to all be vertically and horizontally centered. Easiest way I've found is to use flexbox, but there are people doing it with grid, and obviously it'd be possible to simulate with %, though it'd be harder and possibly less user-friendly.
The style guide includes a width for the designs, which have all been 375px for mobile and 1440px for desktop so far. Your content (other than the attribution) is 417px wide in total, and does crush well to fit the minimum but outgrows the desktop design. Designs can be opened in an image editor to "reverse engineer" many measurements, or opened in an image preview and carefully placed to lineup with the browser window by using alt + tab to switch between the browser and image. I've been eyeballing it, but measurements would be more accurate of course.
The font-family should include a generic fallback in case a user block Google Fonts or it doesn't load for any reason.
Repeated colors, e.g. hsl(0, 0%, 100%) would be more readable as named CSS vars.
Marked as helpful
@atulpalkar16
Submitted
@l3laze
Posted
This looks awesome, and the animations are a great touch. I didn't expect them, so after I moused over it I had to mess around for a moment to explore. Lol. I also like that you used grid; I've been having a lot of difficulty getting that, and was hoping to see it used on this project. Are there any links you could share to help gain a better understanding of grid usage?
The only issue I can see is not having consistent formatting for readability. A lint tool or formatter would help.
HTML
CSS
Excessive selectors, e.g. h1, h2, h3, h4, h5, h6.
Capitalization on Font-size.
Inconsistent whitespace throughout: between selectors and opening brace ({), between blocks (both excessive newlines and lack of newlines), between property and value (prop: value with more than 1 space), between end of declaration and ending semicolon (;).
Marked as helpful
