Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • Lefty 20

    @Lefty3

    Submitted

    Spacing the columns to align both horizontally was a challenge, but googling and reviewing others similar designs helped.

    raionus 310

    @sinjin25

    Posted

    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 :)

    0
  • Rares-29 50

    @Rares-29

    Submitted

    I don't know if I did the javascript part right, it is my first project of javascript. More exactly this part of code: submit.addEventListener("click", function(){ document.querySelector(".selector").innerHTML = "You selected " + stars + " out of 5"; document.querySelector(".div-boss.first-div").classList.add("hidden"); document.querySelector(".div-boss.thank-you").classList.remove("hidden"); }), where I should display the thank you page.

    raionus 310

    @sinjin25

    Posted

    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

    0
  • SpartanUI 245

    @Unal-Inanli

    Submitted

    I'm trying to solidify my knowledge and application of B.E.M class names and I was just wondering if you also find it difficult to just keep on finding the most descriptive name for that element. Is there a technique that will push me in the right direction?

    raionus 310

    @sinjin25

    Posted

    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

    1
  • raionus 310

    @sinjin25

    Posted

    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

    0
  • raionus 310

    @sinjin25

    Posted

    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

    2
  • Dan 30

    @txhawg

    Submitted

    OK, 2nd try at this challenge. I think I have a better understanding of some of the concepts and what the challenges require.

    raionus 310

    @sinjin25

    Posted

    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.

    0
  • raionus 310

    @sinjin25

    Posted

    Good attempt :)

    If you look at the design doc for mobile, you can see that the image is actually cropped on mobile sizes. You can do this for mobile by setting a height on the img and using object-fit:

    object-fit: cover;
    max-height: 200px;
    

    As for the colors and fonts, there is a 'style-guide.md' provided in the files you downloaded if you want to get the fonts and colors as they were in the design doc. You're also free to explore alternative styles as well

    Marked as helpful

    0
  • raionus 310

    @sinjin25

    Posted

    Good job

    I think your image is broke on the github deployment.

    I was able to fix it on github by changing the url: url(..icon-view.svg) to url(icon-view.svg)

    A bit of padding to the right of the avatar and a bit less margin on top of the decorative bar might get it a little closer

    Marked as helpful

    0
  • raionus 310

    @sinjin25

    Submitted

    Vue solution abusing slots. Used grid instead of flex on the chart itself because I designed it last (so I had I knew the height it had to be ahead of time).

    Questions: Any feedback on how to modify the size of the svg while keeping the aspect ratio? I literally just modified the height and width which didn't exactly give the results I wanted.

    raionus 310

    @sinjin25

    Posted

    Oh man I didn't actually finish. How do I delete this :(

    EDIT: decent attempt after finishing the desktop version.

    • double border-radius on card elements
    • svg size is off
    • "my balance" font-size slightly too large (use padding/margin instead)
    • bottom-right text slightly too big; use padding to reduce the height of the div instead
    • bar labels slightly too small
    • add a tiny bit more padding on the bars so they're slimmer
    0
  • EJ 100

    @ejionaut

    Submitted

    I'm still currently playing around with grids. For this exercise, I am not sure why the container wasn't wrapping when I adjusted the size. So I just set the column flow to row when adjusted.

    Any feedback is welcome!

    raionus 310

    @sinjin25

    Posted

    Good job, especially on the the padding between card sections. I messed that up on my solution.

    I think that your font-size is slightly too big on the body text and the button text is slightly too small. Very good work though.

    You might want to use a position: fixed on the attribution text so that it's taken out of the document flow. This will help your solution line up with the design on the design comparison

    Marked as helpful

    0
  • raionus 310

    @sinjin25

    Posted

    Good job on the text!

    You can use the letter-spacing attribute on the "perfume" text to make the letters appear more far apart like in the design

    You can use flexbox with align-items: center on the add cart button to vertically center the cart and text.

    The primary price tag (not the crossed-out one) should be the same font and the "Gabrielle..." title text.

    0
  • P

    @Creixz

    Submitted

    Hello guys, I would like to know some specific points and I hope you can help me.

    1. Did I correctly implement the BEM methodology?
    2. Is there any way to improve or optimize my code? Thank you for your time and best wishes to you.
    raionus 310

    @sinjin25

    Posted

    The BEM seems correct.

    I'd recommend putting padding on the card rather than on each element inside the card. This will let you standardize the padding instead of having to set in everywhere. The method you did makes it hard to make changes.

    You can get away with just having this on the image:

    max-width: 100%
    border-radius: (whatever)
    

    That lets you get away from using a magic number for width.

    grid-template-rows with percents seems like a bit of a fragile solution. I'd recommend splitting it like: card -> card-head + card-body and then maybe using fr to divide up the space between the head and body.

    Marked as helpful

    1