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

  • Cássia• 90

    @casbern

    Submitted

    Hi guys!

    Is there anything I can improve on my HTML, CSS e JS? How to make it shorter just to not repeat myself so much in my code? I tried to make it as concise as I could. I just do not know if I am in the right direction.

    Thank you very much in advance.

    Mobile first page with CSS, JS, BEM, Semantic HTML5

    1

    Sean Dickinson• 135

    @atinybeardedman

    Posted

    Hi there, I think there's a few things you can do to improve this one. I wouldn't say the goal is always conciseness, but rather readability. Concise is great, but being able to read and understand code is better. So here are some suggestions/general feedback:

    • In javascript try to avoid comparing things directly to boolean values in a condition. Instead just use the boolean directly. So in your case you can just check if(checkedValue.checked){...}. Along those lines I would consider a clearer variable name. So in your case the "checked" state means they want monthly amounts. So I would name it something like isMonthlyCheckBox or something like that so it's super clear what the logical value is associated with. Also in here you have a minor bug with the array indexes. You are making the pro plan cheaper than the basic plan in the js.
    • I think your css BEM style looks really good here. You have clearly distinct namespaces for the different sections they are easy to read and understand.
    • Your html markup looks good. These buttons don't do anything so it's a toss up if they should be buttons or a tags, but you have aria role button on the a tag which might actually be a bit confusing to screen readers since they already pick up a tags on their own. I might just stick with an a tag without the role button, but I'm not an accessibility expert.
    • You have a minor reflow issue when you hover the middle button on desktop. This is because the border doesn't exist on the non-hovered state, so the 2px that is added (1 for top and 1 for bottom) is enough to visibly shift the layout of the card. Not a big deal, but easily fixed by adding a 1 px white border to all buttons so that it's always there are really you are just changing the color on hover. In terms of these color changes, I'd highly recommend adding a css transition on the btn class for color, border-color, and background. It's subtle but it brings up the percieved user experience a bit.

    Overall this looks great, nice work!

    Marked as helpful

    3
  • Sean Dickinson• 135

    @atinybeardedman

    Posted

    This looks good. Here are a few suggestions:

    1. I think you could swap out a div for a span element for your logo. If you inspect it, you'll notice the child svg is actually expanding outside the span since the span element is inline and doesn't calculate it's width and height based on it's children the same way as a block level element.

    2. Your mobile solution doesn't have the white space around the cards as shown in the preview. A better solution than using flex-wrap is to explicitly change the flex-direction to column when you hit the mobile breakpoint. Then you can set a max-width to 90% or something similar on the cards to get that padding.

    3. I don't think you should be adjusting the height manually for the mobile breakpoint, rather just adjust the padding. Setting explicit heights for something like this is bad practice in case the content changes and then you run into overflow problems.

    0
  • Sean Dickinson• 135

    @atinybeardedman

    Posted

    I think you have a typo for the font-family. It should be "Big Shoulders Display" not "Big Shoulder Display". Also I would recommend always adding the fall-back font that google suggests for google fonts in the font-family definition. It's always important to have that there in case the remote font doesn't load correctly.

    Marked as helpful

    0