Skip to content
OverviewSolutions
17
Comments
9
JunoField
@JunoField

All comments

  • JunoField•310
    @JunoFieldPosted about 1 year ago

    Hello, congrats on your first project!

    Here's some tips that may help you for your next project:

    • The layout does not work on mobile devices. Generally, you should code the mobile version of the site first, then use media queries to make modifications for the desktop version. Thankfully, the differences for this project are very minimal.
    • The sizes and colours do not match with the design. Colours can be found in the style-guide.md file.
    • As for sizing, I recommend zooming to 100% on the design files then use a tool (such as PowerToys Screen Ruler, if you're on Windows) to measure sizes.
    • Finally, I see you're currently using the default font and not the one recommended by the style guide. Thankfully, adding a font from Google is very easy: here's a tutorial.

    Good luck with your next project!

    0
  • JunoField•310
    @JunoFieldPosted almost 2 years ago

    Hi,

    This was a great attempt, just a few suggestions:

    Some of the CSS "tweaks" you've added aren't necessary for this project - sections 7 and 8 for example. There's no real need to remove these - just thought I'd make you aware.

    Ideally the main element should be centered - another commented mentioned a grid solution, but personally I'd just use absolute positioning. This can be done by adding this to the centre card's CSS:

    position: absolute;
left: 50%;
top: 50%;
transform: translateXY(-50%, -50%);

    Also, take note of the issues raised in the FM report:

    • Alt text: Personally I'd either make it empty or add a link, but neither are ideal solutions in production so it's really up to you.
    • Main tag required: This one's simple: just change your section tag to main and update the CSS to suit.
    • Attribution section: Personally I'd replace div with footer for the attribution to fit this requirement.

    Speaking of the attribution section, don't forget to fill it out with your name and profile. I also like to move its styling from the HTML file to the CSS file but that's down to preference.

    Good luck!

    0
  • JunoField•310
    @JunoFieldPosted over 2 years ago

    Hi,

    Your CSS and images are not linked as of right now. To fix this, simply change href="/styles.css" to href="./styles.css" (with a period before the slash), and the same for all your images.

    Good luck!

    Marked as helpful
    1
  • jwalczak94•810
    @jwalczak94Submitted over 2 years ago
    What are you most proud of, and what would you do differently next time?

    I would probably use classes or IDs to select individual elements.

    What challenges did you encounter, and how did you overcome them?

    Nothing, it was an easy challenge.

    What specific areas of your project would you like help with?

    I don't need a help with this project.

    QR code component using Flexbox

    1
    JunoField•310
    @JunoFieldPosted over 2 years ago

    Hi,

    Congrats on your first project - you've got it looking very close to the design with 0 HTML/accessibility faults!

    The only things I'll mention are:

    • On mobile the card is not centred in the visible screen area and requires scrolling - it should be reasonably easy to centre it though.
    • When only styling a single element, use IDs and not classes. Classes are typically used when you want multiple elements styled consistently.

    Good luck!

    Marked as helpful
    0
  • JunoField•310
    @JunoFieldPosted over 2 years ago

    Hi,

    I'm assuming you intended to centre the .main-container component vertically as well as horizontally. If this is the case, simply add this to its CSS on the desktop version:

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

    Also, I noticed that you only used classes and no IDs for your HTML/CSS. The general rule is to use classes when they are applied to multiple elements, and IDs if they are for a single element - which is the case for most of yours.

    Hope this helps!

    Marked as helpful
    0
  • Lukas•150
    @wanlukSubmitted over 2 years ago

    How can I fix the ratings? Specifically the width of their background.

    Social proof section

    1
    JunoField•310
    @JunoFieldPosted over 2 years ago

    Hi,

    It seems that while trying to offset the review sections horizontally, you have accidentally offset the contents inside them instead by using justify-content. Fortunately this is very easy to fix - simply remove that line and add a margin-left instead.

    In terms of their width, if you want to add a fixed width simply add width: ___px to their CSS.

    Hope this helps!

    0
  • JunoField•310
    @JunoFieldPosted over 2 years ago

    Hi,

    Congrats on completing the project. Some things that come to mind:

    • On the "Improve your fr..." section, your font size is much too large. Just decreasing this would make a big difference to the page's appearance.
    • Your centre card and QR code currently have black borders - to disable this, simply add border: none; to the CSS for those sections.
    • Element IDs should be named descriptively - names like wrapper are a big no-go. Instead use names like centre-card, text-section, etc.
    • There's a few HTML and accessibility issues - I'll leave you to work those out, as it's good experience to solve those yourself (with the help of Google).
    • Once you've made these main changes, play around with margins, padding, etc. to get it looking as close to the design as possible.

    Good luck!

    0
  • JunoField•310
    @JunoFieldPosted over 2 years ago

    Hi,

    On mobile your centre card is far too narrow - I have a feeling you may have made a typo and written 200px instead of the intended ~300px.

    In case you're not aware, you can press Ctrl+Shift+M (and F12 first if using Chrome) to preview the mobile version, if that helps.

    Good luck!

    Marked as helpful
    1
  • JunoField•310
    @JunoFieldPosted over 2 years ago

    Hi,

    "The design was created to a width of 1440px" does not necessarily mean this will be part of the code. Instead, it means that the design screenshot they provide was made to a width of 1440px. So, if you're aligning things very precisely, you would want to preview the page with a width of 1440px.

    Some other advice:

    • There's no mobile site. You'll want to look into "media queries" to ensure that the page responds to the window size.
    • When you're comfortable with media queries, you'll want to make the mobile design first then move onto the desktop - the reason being that on a phone, the desktop version won't be unnecessarily loaded, wasting resources.
    • In this project, the central card should be a fixed width - this will mean the differences between mobile and desktop are very minimal.
    • Element IDs should be named descriptively, i.e. text not second-child.
    • The original readme should be removed/renamed, then the README-template.md edited and renamed to README.md.

    Overall, this is a good starting point. Good luck for future projects!

    1