My new website about hip hop(franklinsweekly.com)

almost 6 years ago from William Bengtsson, Product designer

  • Thomas Michael SemmlerThomas Michael Semmler, almost 6 years ago

    Well that looks nice! So happy to see something creative, meaning not religiously and blindly following every trend. I see them there, but at least I don't have to look at jazz shapes and pink/green ferns with playfair display over them.

    Someone already got you feedback on your performance issues and also tried to sell the fixes to you. So here are some things you can do right now, to improve your codebase:

    • all your images need an alt attribute
    • the width-attributes of images only accepts pixel values. That means, that you can omit it. Just writing width="100" is enough.
    • Don't use an image for text. The times where we need to embed images for texts is over. You can instead style everything with CSS, or if that seems to complicated for you, take the whole text, import into sketch or illustrator or any other application that supports svg and convert it into outlines and embed this svg. You can directly embed svg code into your html. Why is that better? SVG is almost always better, and you have a baked in accessibility win with the title and desc elements.
    • now, your outline is unfortunately really messed up:

    outline

    there is one very simple rule about headings - they have to start at <h1> and you cannot skip their order. You cannot just use an <h5>, if it is not preceded by <h4>.

    • in html, there are elements that do not wrap any content. Those "empty" elements consist of one self-closing tag. In XHTML we used to write <img /> in html5, you should write <img>. Doing either way is actually fine, but I suggest you get used to doing it the "correct" way. One of those empty elements is the <br> element. It causes a line break. You cannot wrap it around other content, as you did from line 64 to line 152.
    • even thought it works, you should also try to avoid elements outside of the head. There are three ways to apply css: linked stylesheets, css in the style element in , or inline style. Try to avoid it outside of these three ways.
    • you have a lot of duplicate id's in your svg code. If you find yourself reusing svg over and over, you should make use of <symbol> and <use xlink:href="">. ID's can only occur once per dokument, and an element also can only have one ID. This is very important to respect.
    • you have a lot of stylesheets in your document in general. You should try to avoid @import at all cost and instead concatenate your stylesheets locally into one stylesheet.
    • you use roboto and lato from google fonts and you use one <link> for each of them, which means, two requests. You should instead combine them: https://fonts.googleapis.com/css?family=Roboto|Lato
    • your .backgroundHero is a background image. This is ultimately up to you, to decide weather an imag should be a CSS background image or an <img>, BUT, the people on the background-image are not just decoration, are they? They are part of the content. And if you want an image to be content, it needs to be <img>. So you should try and make this layout with individual images. This is not very problematic to do. I suggest you try out CSS grid. You can still fallback with the background-image for browsers that don't support it.
    • You load jquery twice, which is odd.
    • I suggest you get rid of foundation entirely. Not because its bad, but because your layout is very basic and very easy to do. There is no reason to use foundation or any other framework, or animate.css for that matter - it just costs you a lot of bytes.
    • you can however still use them, if you use the scss versions of those libraries. That way, you can only use what you need and you omit the rest.

    Good luck!

    5 points
    • William BengtssonWilliam Bengtsson, almost 6 years ago

      Hey Thomas, I'm sincerely grateful for your feedback. Thank you very much! Great that you wrote such detailed feedback, I believe I can fix all of these now because of that, so thank you. I'll probably skip Foundation next time since there's literally hundreds/thousands lines of CSS I don't use, so that will be the next quest. (The outline image you attached doesn't load for me for some reason? But I suppose it's an SVG that hasn't scaled properly due to being borders, which I'll have to fix).

      Thank you again Thomas, I'll look into all of this. Greatly appreciate it!

      1 point
    • Joe AlfonsoJoe Alfonso, almost 6 years ago

      Damn Thomas, this is great. I read this all just for personal learning.

      1 point
    • William BengtssonWilliam Bengtsson, almost 6 years ago

      Thank you again Thomas for all these tips and recommendations. I've followed up some, some I'm still working on. Understand you're not perhaps interested in follow-up but just wanted to drop by my updates anyhow :)!

      • Added alt on images

      • Added a sitemap.xml and a robots.txt

      • Replaced one image that was text, but the big one on "about" I still have problems since the div with card+text goes underneath the profile image, something I'm working on.

      • I've used < h1 > to < h6 > just to make them go in different font-sizes, I might need to change this later but have to create a system that differens than when there's a title

      • I'm trying to locate all the CSS I've used from the foundations.css template and insert into my own css-file to limit the css amounts. I've removed 2 CSS-files and now only use two (from previously 4). Removed about 800 lines of code.

      • I've searched and replaced the @import that I could find

      • Updated so that it said Roboto|Lato instead of two urls to each font family. Then realised I don't need Roboto so removed it altogether and only using Lato.

      • I can't find why jquery is loaded twice, but if I remove the jquery from the far bottom, jquery doesn't load for me at all. I believe it's the import of the Newsletter section that loads jquery but can't figure out how/why

      • Next project I'm doing is based on SCSS, I'm getting into it now, will look into if I can redo this into SCSS when I feel more secure about it.

      0 points