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

  • CaplexW• 60

    @CaplexW

    Submitted

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

    For some reason, the fonts on my page are displayed differently than they did in the design photo, despite the fact that I used the same fonts and weight that were given in the style guide. I don't know why and how to fix it.

    P
    Will• 310

    @wansmth

    Posted

    Nice work.

    The font looks like it has some opacity on it - that'd explain the difference you can see. You can fix it by either changing the alpha value of the font colour, or changing the opacity property of the container.

    Marked as helpful

    1
  • P
    Will• 310

    @wansmth

    Posted

    Hi Diego,

    Nice solution, it looks great.

    Looking through your solution, the only thing I could think of which might help in future is: for the background image on the purple card, I think it's easier to use the background image property. This also has the benefit of keeping the content of your page separate from the styling.

    Hope this helps!

    Marked as helpful

    0
  • P
    Will• 310

    @wansmth

    Posted

    Hi,

    Nice work! I looked through your solution to try to find some things to improve on and came up with these few things (although they are a bit nit-picky):

    If you look at your Google Fonts import, you've actually listed a bunch of fonts to import (the ones from the previous challenges). I noticed I'd done this too. Google Fonts has a 'basket' of fonts which isn't cleared upon refreshing the page. I looked into this and modern browsers should avoid downloading imported fonts which haven't been used, so it's not really a problem but worth noting I suppose.

    I would have personally made a CSS selector the 'card boxes' in general. This would have saved you repeating a large amount of properties for each of the properties. It is really only the grid information which is specific to a given box.

    Hope this was of some help!

    Marked as helpful

    0
  • P
    Will• 310

    @wansmth

    Posted

    Hi Diego,

    Your solution looks great. Some things I think you could improve on though are the following:

    You have styled the "Perfume" heading by directly adding spaces and capitalising the letters in the HTML. There are properties in CSS to do this: 'letter-spacing' and 'text-transform'.

    I am not sure if it is a result of using React, but I think you could choose closer-matching HTML elements. For example the table row you have used to contain pricing information, it's not really tabular information and so a simple div with either flexbox or grid might have been better suited. The main argument for this approach would be accessibility, as a screen reader announcing a table is a little bit inaccurate.

    Hope this was somewhat useful.

    Marked as helpful

    0
  • P
    Will• 310

    @wansmth

    Posted

    Hi,

    I'm not sure where I got the shadow on the QR code in all honesty lol. I put one on my solution but I think I meant to put it on the whole card itself. Looking at it now, there seems to be a subtle shadow at the bottom of the card, so it looks as if it is raised from the page slightly.

    Either way, it's just one of these: https://developer.mozilla.org/en-US/docs/Web/CSS/box-shadow

    If you Google 'css box shadow' there are websites which allow you to visually tweak a box shadow and then copy paste the css over.

    Sorry for the confusion.

    For the centring, I was doing this:

    body { height: 100vh; display: flex; align-items: center; justify-content: center; }

    Marked as helpful

    0
  • P
    Will• 310

    @wansmth

    Posted

    Hi! Nice work. Just as some positive criticism, I would say you could perhaps look at choosing HTML containers with closer matching semantics. For example you used an unordered list for the nutrition information at the bottom of the page. You have created a table-like layout using an unordered list but you could have used an actual HTML table. This would make it more accessible for screen readers, as well as arguably making the HTML easier to read.

    Another tip might be to look into using variables in your CSS for things such as the colours. They are arguably not necessary here but it might make the code more readable if you are using the same colour in multiple places (for example, seeing the colour variable 'main colour' in multiple places, it is easier to recognise that those elements use the same colour, as opposed to seeing just a hex value e.g. #FF2B11). Of course there are other benefits, too.

    Hope this was of some help at least.

    0
  • P
    Will• 310

    @wansmth

    Posted

    Hi, nice work. The colour transition on the buttons looks good.

    A few areas to make note of and improve on in my opinion would be to perhaps structure include enclosing containers for the different sections of the card - i.e. a nav container around the buttons and another container around the personal information parts. This would make it more accessible as it would be easier to quickly navigate to different sections and then subsections using a keyboard. It would arguably also be more readable at a glance.

    Marked as helpful

    0
  • P
    Will• 310

    @wansmth

    Posted

    Looks good to me. If I had to nit-pick a few things:

    • Missing the shadow on the QR code image.
    • The card isn't vertically aligned with the page and so does not stay centred when you zoom out. Consider making the body element 100% of the viewport height and using flexbox to vertically align, rather than using top-margin as you have.

    Marked as helpful

    0