Skip to content
OverviewSolutions
49
Comments
18

Brendan

@OneManBannedEngland1,120 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

I’m currently learning...

Typescript, NodeJS, Linux.

Latest solutions

View more solutions

Latest comments

  • Brendan•1,120
    @OneManBannedPosted over 1 year ago

    Hello Benjamin,

    • if you add min-height to the body it will center everything on the page.

    Nice solution.

    1
  • Brendan•1,120
    @OneManBannedPosted over 1 year ago

    Hi Alejandro, nice work..

    I have a few observations.

    • You don't need to add alt to purely decorative images, and the background images would probably be better if they were a css background image, instead of cluttering your html.

    • Perhaps the h2 would be better in a dl link. The description list element key-value pairs. Something like this ...

    <dl>
<dd>followers</dd>
<dt>80k</dt>
<dd>likes</dd>
<dt>803k</dt>
<dd>photos</dd>
<dt>1.4k</dt>
</dl>

    Happy coding,

    Brendan

    Marked as helpful
    1
  • Brendan•1,120
    @OneManBannedPosted over 1 year ago

    Hi Aisha, here is some answers and advice...

    • 1 - You currently have your img inside the span tag and the h4 outside. It would be better if you used this structure
     <li>
<img src="./assets/images/icon-success.svg" alt="" id="icon">
<p>Product discovery and building what matters</p>
 </li>

    And then wrap all three li tags in a ul tag. Although, even then the image and the text would'nt be on the same line. Because <p> is a block level element. In order to get them on the same line you could declare.

    li {
display: flex;
}

    • 2 - The font size is being applied to the <h1> for me.

    • 3 - I think you could probably find a good tutorial on YouTube for this.

    • 4 - The easiest way to link your success.html would be to take everything inside the success.html <body> tag, wrap it in a <section> tag and place it into your index.html above the closing </body> tag and then toggle display in your javascript.

    -5 - A combination of linear-gradient and box-shadow are used to create this effect. (I['ve linked their mdn pages) and here is useful tool to create box shadows - link.

    • 6 - The span is colored it's just a very dark blue. Try adding a brighter color.

    • 7 - I would suggest completing more newbie challenges on frontend mentor. They let you concentrate on small but important details and after a few you will feel a lot more confident tackling the tougher challenges.

    Hope that helps. Happy coding!!!

    Brendan.

    Marked as helpful
    0
  • Brendan•1,120
    @OneManBannedPosted over 1 year ago

    Hi Vishal,

    It's visible for me on mobile mode.

    It maybe a problem with zoom. When you're looking at in responsive mode try pressing ctrl + 0(On windows). to reset the zoom.

    Here are some observations on your solution.

    HTML

    • The .main container <div>. Should be a <main> landmark element. Landmark elements help to give your page structure.

    • Your li tags don't have closing tags and they all need to be wrapped inside a ul

    • The <h6> tag should be a <label> connected to the <input> like this ---

    <label for=email" />Email address</label>
<input type="email" placeholder="email@company.com" name="email"</input>

    here is the mdn page on labels and how to connect them to an input.

    Happy coding, Brendan

    Marked as helpful
    1
  • Brendan•1,120
    @OneManBannedPosted over 1 year ago

    Hi Rabel, I have a few suggestions for you.

    • The <h3> tag should be a <h1>. Every page needs a <h1> and the heading elements (<h1><h2><h3><h4><h5><h6>) must be used in ascending order. Here is the mdn page on heading elements

    CSS

    • If you remove the margin declarations from .card and the width from main. Then add the following to main
    justify-content: center;
min-height: 100vh

    you will have it all nicely centered using flex-box :)

    • I suggest you look at using relative units instead of pixels. Have a look a rem, em, vh, vw and %. Here is a handy page that converts pixel vales into rem (link)

    Nice solution. Happy Coding

    Brendan.

    Marked as helpful
    0
  • Brendan•1,120
    @OneManBannedPosted over 1 year ago

    Hi Igor, very nice looking solution. A few bits of advice.

    HTML

    • Very little to say here other than I think the change link would make more sense as a <button>. The <a> element is used to link to other webpages while the <button> element is used to trigger a JavaScript action. Here, I assume in fully working app pressing "change" would just change the details of the annual plan.

    • I also don't think you need an alt="" tag on the image. Images that are purely decorative or are described in text nearby don't need alt tags.

    CSS

    • A little margin-block: 2rem on .container would push it away from the top and bottom.

    • I would suggest not using px so much. Try relative units instead %, rem, em, vh, vw. They will help with responsive pages.

    • Also try avoiding setting fixed heights on elements. Use padding and margin instead to reach the desired height. This also helps with responsiveness.

    Well done, look really good.

    Marked as helpful
    1
View more comments