Skip to content
OverviewSolutions
10
Comments
14

raionus

@sinjin25370 points

I like rouge-likes, indoor rock climbing, and vtubers I guess. Worked on Koohi.cafe, a Japanese-learning site and other things. Discord: raionus#8576 Looking for work xd

I’m currently learning...

Test driven development & learning React (coming from Vue)

Latest solutions

View more solutions

Latest comments

  • raionus•370
    @sinjin25

    Hey Lefty, nice job on the project.

    One thing that'll help you out a lot going forward is making sure you've got your fonts right. This is a big deal because different fonts have different heights which makes it difficult to get your design to match up with the design docs. The process is not very obvious when the fonts don't come on your system either so that tricks up a lot of people :(

    1. Check the style-guide.md. It'll say the font family and the weights required.
    2. Go to google fonts and search up the family (ex: https://fonts.google.com/specimen/Lexend+Deca?query=lexend+deca)
    3. Select the font weights required (400, 700)
    4. Follow the instructions on the right so:
    <link rel="preconnect" href="https://fonts.googleapis.com">
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Lexend+Deca:wght@400;700&display=swap" rel="stylesheet">

    is placed inside the <head>

    and font-family: 'Lexend Deca', sans-serif; can be added as a CSS rule to .body {} (or anywhere you want).

    You aren't told very well if you're using the font correctly. On firefox you can go inspect a text node -> check for "fonts" (near computed) -> make sure the font family is correct and not falling back to times new roman or something. On chrome you can inspect a node. Checkout the "computed" tab and scroll down to the bottom where it says "rendered fonts". Make sure it's the correct one and not a fallback.

    Good job and good luck on your next projects :)

  • raionus•370
    @sinjin25

    The js seems fine to me at least.

    I think that your script itself could be improved. I would recommend putting it in the head then using the defer attribute. This is non-blocking (according to https://www.w3schools.com/tags/att_script_defer.asp) and waits until the dom content is loaded to execute, so no need to put it in the body anymore.

    Your loop readability might be improved by using a .forEach but that is up to personal style.

    Good work on the project :)

    Marked as helpful
  • raionus•370
    @sinjin25

    I also find BEM naming to be kind of tedious (still useful though). If you're running out of brain power for names maybe you could try double modules?

    ex: instead of expense-component__balance-title and expense-component__balance-amount

    expense-component__balance__ and then you'd have the submodules title and amount. Maybe then you could be undescriptive while still being confident you avoid a naming collision?

    I noticed you're not using sass. If you're using BEM it can make it far less horrible through nesting. Instead of writing out the full module for everything:

    .expense-component__ {
    &balance-amount {}
    &balance-title{}
    &stats-container {}
    &stats-end {}
}

    etc.

    Marked as helpful
  • raionus•370
    @sinjin25

    Q. Should I have used Id's instead of classes??

    No I think classes are best in this case. Ids should only be used once per page, so they reduce the reusability of anything you make.

    I mostly just use them to target things easily in javascript. They can be used for styling but I think using BEM naming is more flexible.

    Q. Should I have used a section tag in my html file instead of main??

    I think main is fine. <section> is part of semantic html. Sections should usually have a h1/h2/h3 inside of it or I believe it's considered an accessibility error. Since you don't have headers, it would be inappropriate I believe.

    Q. How do you link fonts directly into the Css file using import??

    If you're using google fonts (which is best for these challenges), you can copy and paste their code directly into your css file. The @import should be by itself. The font-family should be inside a rule (probably body). I attached a screenshot of where it is on google

    https://gyazo.com/e2334adb17e22469a81d058149245a06

    Q. Any other general tips would be much appreciated.

    I like using this rule for this website:

    position: fixed;
left: 50%;
top: 50%;
transform: translate(-50%, -50%);

    I add this right before I submit. It should center whatever you put it on (the container, card, etc.). This helps line up your work for the design comparison.

    I would 100% avoid putting styling directly on elements (ex: your main main img etc.). There aren't any issues here, directly styled tags are a nightmare. Any styling you do in the future has to specifically override the default tag styling you've added.

    I'd also recommend using a reset.css file (you can find some on google). This helps reduce the variability between how your work looks on different browsers by removing most/all the styling from all tags.

    Marked as helpful
  • raionus•370
    @sinjin25

    I think that this piece of code is pretty fragile: rating=ratingButtons[i].innerHTML because if someone changed the html structure it would fall apart.

    You could try textContent or even more reliable would be to use an arbitrary attribute (I used data-value="3") to store it, that way the value is independent of the labeling.

    For your event handling. I think you're using the old style of event listeners (not sure). I think the most commonly accepted way right now is using event listeners. So someEle.addEventListener('click', () => { /* do something */ })

    Marked as helpful
  • raionus•370
    @sinjin25

    Good job, you're really close to perfect.

    It looks like the design has slightly more padding and the title text is blue.

    Your font-size is a little too big.

    One trick I like to use is counting how many words are on the last line of the text. For instance, in the design the last line is "next level" while yours is "the next level" which tells you that your font-size isn't quite right. This doesn't always work but is easier to do than eyeballing it.

View more comments