Skip to content
OverviewSolutions
13
Comments
19
Purnima Kumar
@purnimakumarr

All comments

  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Felicia, congrats on completing the challenge successfully 🎉

    Here's how I think you can improve it just a little bit more, you can add cursor: pointer property on the Submit Button so when someone hover over it they see a pointer, it will make the fact more profound that the button is clickable. And I suggest the Cancel Order element should be a button and Change element should be a link rather than just text beacuse the active state suggests they are clickable.

    1
  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Jarrian, your solution looks really good. You can improve it a little bit by adding transition: all 0.3s on the .nft-hover,.title and .name elements. This will make the hover effect smooth. Sometimes, little details like this really makes a difference!

    Marked as helpful
    0
  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Lucas, your solution looks really good. There is some gap below the image, which you can see when hover over it, this is beacuse the <img /> is an inline element. You can get rid of that by using display: block on the <img />. Other than that, great effort!

    0
  • Mr Skyner•30
    @MrSkynerSubmitted over 2 years ago

    Hello! This is my solution for the project named Stats Preview Card!

    Any feedback it's helpful! Thanks for checking and feedback!

    Stats Preview Card

    3
    Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Skyner, your solutions look really good. But it isn't centered properly. Here's what you can do..

    body {
    height: 100vh;
    display: grid;
    place-items: center;
}

    And get rid of the margin on the card component.

    1
  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hey strael12,

    I would suggest a few changes to make you solution better:-

    1. Right now you have width: 37% on the .container-form. I would suggest setting the width in px instead of % beacuse right now the card's width is very less for screen sizes 1024px to 1440px. You can use max-width: 700px. This way the card's width does shrink appropriately for smaller screens but doesn't expand for bigger screens.

    2. You can add a border-radius: 9px 0 0 9px on the card image to get rounded edges just like in the design.

    Marked as helpful
    1
  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Riddha, congrats on completing you first challenge 🙌

    Here are some suggestions to make you code more accessible:

    1. Using an <h1></h1> tag. It is a good practice to use heading tag for the heading in a website and as the name suggests an <h1></h1> is used for the very first heading of the website. So, you can replace <p class="desc-heading">Improve your front-end skills by building projects</p> with <h1 class="desc-heading">Improve your front-end skills by building projects</h1>

    2. The alt-text="qr-code" isn't very descriptive. alt-text attribute is used to describe an image so that users using screen readers can know what an image is about or what is the purpose of using the image. Something like alt-text="A QR code to Frontend Mentor" would be better.

    0
  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Akash, your card component looks pretty close to the design. I see you have only used the bg-pattern-top for the background. But what would be more accurate to the design is that you use both bg-pattern-top and bg-pattern-bottom. You can specify them as two seperate images using the img tag and then position them as specified in the design using position: fixed or position: absolute along with top, bottom, left and right to define the position.

    And you can replace the width: 25vw with max-width: 25vw. Right now, the card component expands in width but the content doesn't and thus it doesn't look good on bigger screens.

    Also, I think you can improve the alt-text for the images used for header and avatar. alt-text hepls users who use screen readers to know about an image, so alt-text should be descriptive. Here, the header image is used only for decorative purposes so you can use alt-text="". As the header image doesn't contribute anything to the actual content of the website we can set an empty alt-text, in this way screen readers will consider it as a decorative image and ignore it. And for the avatar, you can use something like this alt-text="Victor Crest wearing a white shirt and a black jacket" This makes your code more accessible.

    Marked as helpful
    1
  • Joy•80
    @joykaraSubmitted over 2 years ago

    Are there any other best practices for handling the hover in CSS? What else could I use other than the <div> classes in HTML?

    NFT preview card solution

    1
    Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Joy,

    Congrats on completing the challenge 🙌

    Regarding you question about hover effect: I reviewed your code and the way you added style for a hover state looks good and acceptable. I would suggest to always use transition property while working with hover states. It makes the hover effect more smooth and pleasent looking. In your code you can add transition: all 0.3s to the .image component and h3 tag.

    Some suggestion by me: I see there is 1 accessibility issue in your code i.e. beacuse you didn't use an h1 tag in your solution. The first heading of any website should always be wrapped by an h1 tag as it makes the the website more accessible. h1 as the name of the tag suggests is used for the first heading a website. So, you can replace your h3 tag with an h1

    Marked as helpful
    1
  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Carlos,

    • There are 3 accessibility issues in your code which can be solved by using HTML5 semantic tags. You can wrap the <section class="card__container1"></section> and <section class="card__container2"></section> in a <main></main> tag and the attribution in a <footer></footer> tag. This will make your code more accessible. So, your code would look something like this:-
    <main>
    <section class="card__container1">content goes here</section>
    <section class="card__container2">content goes here</section>
</main>
<footer>
    <div class="attribution">content goes here</div>
</footer>
    • Also, you can make the Rating Component more interactive by adding an alert in case a user tries to submit the feedback wihtout selecting a rating, stating something like "Please select a valid rating". This will make the component more interactive.
    Marked as helpful
    1
  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hey Kapline, Congrats on completing your first project involving JavaScript 🙌

    I would suggest two changes:-

    • You should re-position the <footer></footer> section. As right now, it seems to overlap the rating component in desktop design. I would suggest not to position the footer using absolute. You can do it by using Flexbox. I see you are using Flexbox to center the contents in body horizontally and you have specified a margin-top on the body to center the contents vertically. However, you can center the contents of body both horizontally and vertically by using Flexbox only. You only have to add justify-content: center css property to the body tag. This is a flexbox property which aligns the content vertically when the flex-direction: column is set. And, you can add gap: 24px to the body tag to have some spacing between rating card and footer. This is what your CSS code would look like:-
    body {
    height: 100%;
    font-family: 'Overpass', sans-serif;
    background-color: var(--very-dark-blue);
    display: flex;
    align-items: center;
    justify-content: center;
    flex-direction: column;
    gap: 24px;
}

.attribution {
    font-size: 15px;
    text-align: center;
    color: var(--light-grey);
}
    • Also you should put the JS script outside the main section. It should be put at the very last in the code and before body is closed.
    Marked as helpful
    0
  • Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Joao,

    I would suggest these changes:

    • There is 1 accessibility issue. You can remove that by replacing the <h2></h2> tag with an <h1></h1> tag. It is a good practice to put the first heading of your site in an <h1></h1> tag. It makes your site more accessible.

    • Also, I would suggest to put the <footer></footer> outside of the <main></main>. Footer is not a part of the main content of the website. So, your code would look something like this:-

    <body>
    <main>
           main content goes here...
    </main>
    <footer>
         attribution goes here...
    </footer>
</body>

    • And I see you are using a <div></div> tag for the QR code image. The issue is that it makes the code hard to read as one cannot know what the div is for unless they observe the CSS file, and also it is not an accessible option because screen readers see this as just an empty div and therefore, people using screen readers can't know that there is a QR code image there. The fix for this would be to use role and aria-label attributes. <div role="img" aria-label="A QR code to frontendmentor.io"></div> The role="img" specifies that the div is being used to render an image and aria-label attribute works as alt-text for the image in this case.

    • Lastly, you can put the CSS in the index.html file in the CSS file. It is good practice to put all the styles in a seperate CSS file.

    Marked as helpful
    1
  • prakhar•130
    @Prakhar-99Submitted over 2 years ago

    Hii I learn many intresting thing in this challenge. please give my guidence regard this challenge it help me to improve my way of handle a nice code

    using css flex and postions properties

    1
    Purnima Kumar•390
    @purnimakumarrPosted over 2 years ago

    Hi Prakhar

    Congratulations 🙌, your solution looks quite good. I would suggest a few things.

    1. The font-family specified on the elements is not correct. You have specified the font-family: "Outfit" on the body but it is not being rendered correctly. It is beacuse of the import statement. The URL that you have specified is not correct. Its is actually an address which you can paste in you web browser and it will lead you to the page of Outfit font at Google fonts website and then you can select this font from there. The correct import statment would look something like this @import url('https://fonts.googleapis.com/css2family=Outfit:wght@300;400;600&display=swap');

    2. Also, you can define transition: all 0.3s on the .nft-img element so when someone hover over the image the transition is smooth rather than sudden.

    3. Lastly, the .nft-name and .author-name elements have hover effects on them as can be seen in the active-states.jpg in the design folder. You can implement that as well to improve you solution.

    Marked as helpful
    0