Skip to content
OverviewSolutions
5
Comments
10
Liam Frosteman Neogard
@FrostemanNeogard

All comments

  • Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    This is great! I love the extra spin animation on the dice, that's a really nice touch.

    There's only one thing I have to suggest. Fetching advice doesn't seem to work on Firefox. To fix this, all you have to do is add {cache: "no-cache"} as a second parameter on the fetch function. That would look something like this:

    const response = await fetch('https://api.adviceslip.com/advice', {cache: "no-cache"});

    This does remove the two second cooldown on Chrome, though that may be unwanted.

    Hope this helps!

    Marked as helpful
    0
  • Taiwo-art•10
    @Taiwo-artSubmitted over 1 year ago

    Kindly advise best practices/more efficient ways to align the surrounding container carrying the QR code image and the texts underneath as i found it a bit challenging.

    I am unsure of the body background color i used. It took a while to find a color that was close to the prototype and I'm still not satisfied. Will welcome suggestions on tools to use to get accurate colours and hex color codes.

    Thank you.

    Responsive QR code component using CSS Grid and media query

    2
    Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    I think this looks quite good visually, but there's quite a bit of improvements that could be made.

    You seem to have a lot of divs where they aren't necessary. A simple way of structuring the card would be like this:

    main
    img
    div
        h1
        p

    It is quite important for semantic HTML that you put the most important content inside a <main> element. Centrering this could look something like this:

    // Card's parent, in this case the body element
body {
    min-height: 100vh;
    display: flex;
    align-items: center; // Aligns vertically
    justify-content: center; // Aligns horizontally
}

    The <main> element's initial display attribute is block, which means the child elements will take up an entire row and display vertically. Now you can simply style the rest of the content accordingly.

    As for the background color, I believe it can be found in the style-guide.md file, along with all the other colors and styling values used.

    Hope this helps!

    0
  • Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    This looks quite good! I really like how the hamburger menu looks on the mobile view. There are some things that could be improved, though.

    There seems to be some inconsistent padding around the header and the first <section> element. You could either put padding on a container above all elements, or simply put padding on every direct child of <body> which can be done with a selector like this: body > *, though I wouldn't recommend this.

    There are also a couple hover effects that are missing, one of which being on the "trending" and "categories" buttons in the navbar. This seems to just be due to a missing href="#" attribute on their anchor elements. There are also missing hover effects on the "Read more" button as well as the smaller articles.

    Another note is the HTML elements used. Your DOM structure looks like this:

    body
    header
        nav
    section
        div
    footer

    This works, though it could definitely be improved. I'd recommend doing something like this:

    body
    header
        nav
    main
        section
            article
    footer

    A wrapper around the entire thing would probably be quite good as well, but the most important thing is to have the <main> element wrapping the most important content.

    Hope this helps!

    Marked as helpful
    0
  • Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    Adding the following styling to your <figure> elements will place them in the correct positions, though you do need to change grid-template-colums: repeat(5, auto) to grid-template-colums: repeat(4, auto) on your container.

    <main>
    <figure class="item" style="grid-column: 1 / span 2; grid-row: 1;">{...}</figure>
    <figure class="item" style="grid-column: 3; grid-row: 1;">{...} </figure>
    <figure class="item" style="grid-column: 4; grid-row: 1 / span 2;">{...}</figure>
    <figure class="item" style="grid-column: 1; grid-row: 2;">{...}</figure>
    <figure class="item" style="grid-column: 2 / span 2; grid-row: 2;">{...}</figure>
</main>

    You can add this styling via CSS or hard-code it into the HTML, though I would probably recommend the former.

    Hope this helps!

    Marked as helpful
    0
  • Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    This looks great! There are just a few small things that could be improved.

    The easiest fix is to add the missing hover effects on the buttons and the "change" text. Secondly would probably be the mobile view. It seems you've set the box's width to 350px and tried to adjust the padding to 38px 22px, though it is retaining the 54px 45px value. This is simply because there is a typo in the media query; it says 38px 22 px rather than 38px 22px. Remove that space and it'll work.

    The final thing would be the background image. The starter files provide you with one image for mobile view, and one for desktop; though it seems only the desktop one is being used here. What you could do is simply change the background image's url of the body in your media query, or what I think is a better solution would be to look into the srcset attribute of the <img> element.

    Hope this helps!

    0
  • Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    This is pretty great! There's only one small thing I'd like to mention.

    Fetching advice doesn't seem to work on Firefox, though this is very easily fixed. When fetching the API, you can simply add {cache: "no-cache"} as a second parameter, like this:

    const response = await fetch(API_URL, {cache: "no-cache"})

    It is in my opinion a bit cleaner to use async await for stuff like this. Here's an example of what that could look like.

    async function FetchRandomAdvice(callback: Function) {
  const response = await fetch(API_URL, {cache: "no-cache"})
  const data = await response.json()
  const adviceSlip = data.slip
  callback(adviceSlip)
}

    Design-wise, I think the only thing you didn't get quite right was the hover effect for the button. It should have a green dropshadow instead of a lighter background color.

    Hope this helps!

    Marked as helpful
    1
  • daniel•80
    @retailescapeartistSubmitted over 1 year ago

    I had a hard time with the header. It still looks like garbo, but I tried. I was unsure of exactly how i wanted the button to open the menu to act. I should've done mobile first design, because my code is a nightmare to look at. I probably could've done it in less code.

    Landing page

    1
    Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    There's definitely some good stuff here!

    Firstly, you are using CSS variables which I see a lot of people miss quite often. The desktop layout is there, and the hover effect on the "New" section works as it should.

    Here are some things that hinder you quite a bit:

    • A lot of your width and height values are hard-coded in pixels. Doing this for properties such as min-width and min-height is quite common (and important), but for layouts such as this, it's gonna be incredibly hard to have stuff display properly that way. You could try playing around with viewport values such as vw (viewport width), vh (viewport height), as well as vmin and vmax.
    • There seem to be a couple things that simply were forgotten, such as importing the font given in the starter code, though I do see that you do have a variable for the font family which is great! There are also a couple missing hover effects and the logo in the top left is missing.

    I can highly recommend designing for mobile viewports first, then expanding to the desktop view after that. I've recently started doing this and I've found it to be quite a bit easier. You might know this already, but you can press ctrl+shift+m in your browser to bring up "responsive design mode" where you can easily scale your window to specific sizes.

    Hope this helps!

    0
  • Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    First thing I noticed was the input fields by the score numbers. These should be plain uneditable text as this is not something a user would input, but rather something the software would output to display how well the user did on something such as an assessment.

    Another thing is the mobile layout. It is responsive, which is great! However, I think it differs from the design a little bit. On mobile, the site should take up the entire viewport, and there should be no rounded corners at the top of the component. This can be fixed by simply changing the border radius on the first section inside of your media query. Keep in mind that you'll need to put !important when changing the border radius since it is already defined in the main css.

    Two more small things; only the second section has a dropshadow, which looks a little odd due to it being slightly visible above the first component. To fix this, you can simply set the dropshadow to be over the entire <main> element instead. The second thing is using CSS variables. When using a color value in multiple places, it can easily get tedious when you want to change that color, since you'd have to manually update it everywhere you've used it. You can use CSS variables to get around this, that way you only need to change one value to have it be applied everywhere.

    Hope this helps!

    Marked as helpful
    0
  • Fawziyyah-hub•130
    @Fawziyyah-hubSubmitted over 1 year ago

    .it was difficult trying the get the correct width and also the colors that were used

    .i don't know yet

    .Yes i do. how can i balance my school n tech life

    bootsrap

    4
    Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    When starting a challenge, Frontend Mentor gives you a prompt to download the starter code which is a .zip file. In this zip file there will be information about the styling used as well as assets such as the QR code image (which I can see you've used.). If you take a look at the starter code again, you'll find a file named 'style-guide.md' which contains the following:

    # Front-end Style Guide

## Layout

The designs were created to the following widths:

- Mobile: 375px
- Desktop: 1440px

## Colors

- White: hsl(0, 0%, 100%)
- Light gray: hsl(212, 45%, 89%)
- Grayish blue: hsl(220, 15%, 55%)
- Dark blue: hsl(218, 44%, 22%)

## Typography

### Body Copy

- Font size (paragraph): 15px

### Font

- Family: [Outfit](https://fonts.google.com/specimen/Outfit)
- Weights: 400, 700

    Here you can find all the colors that you may have missed as well as font information and such.

    As for the width and height of your container, often you don't need to play around with that too much. You'll see that if you just remove the width: 25%; and height: 69vh; properties on #box then the box will still look good. A div will most of the time shrink and grow based on its contents.

    There's quite a lot more I could go into regarding the CSS, but I hope what I mentioned so far helps!

    1
  • Liam Frosteman Neogard•170
    @FrostemanNeogardPosted over 1 year ago

    One thing you could improve on mobile is to take up the entire viewport by having your div with the class name "component" to take up the entire width and height of the <main> element. This could be done in a variety of ways, but I think a simple width: 100vw; height: 100vh; would work in this scenario since there's no other components or padding or margin to worry about.

    Another thing to improve maintainability in case you'd need to change a color here or maybe a font size there is to store a lot of values inside CSS variables. An example of that could look like this:

    // Declare variables with "--{variable-name}"
// Note that you can access variables while declaring other variables,
// for example, you can put primary-light-red and primary-orangey-yellow
// into the example gradient's colors.
--primary-light-red: hsl(0, 100%, 67%);
--primary-orangey-yellow: hsl(39, 100%, 56%);
--example-gradient: linear-gradient(to bottom, red, green);
// 

// Access variables with "var(--{variable-name})"
.component_summary_category.reac p:first-of-type() {
    color: var(--primary-light-red);
}

.component_result {
    background: var(--example-gradient);
}

    Hope this helps!

    0