Skip to content
OverviewSolutions
7
Comments
2096
T
Grace
@grace-snow

All comments

  • Lokesh•120
    @Lokesh8055Submitted 1 day ago
    What are you most proud of, and what would you do differently next time?

    These 2 projects has really helped me to learn the rules that I need to follow for creating a project in frontend Mentor.

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

    Encountered challenge with using boxShadow for the card.

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

    Please go through my project and let me know on the areas that I can improve.

    Blog Preview Card Solution using HTML and CSS

    1
    Grace•31,250
    @grace-snowPosted about 7 hours ago

    The same feedback I've just left on your previous solution applies here so I won't repeat.

    There's one really important extra consideration you've missed in this challenge though: the link. The design shows this card is meant to be clickable, but you've not included the essential interactive element, instead adding hover styles to non-interactive elements. This makes the whole component non-functional (I.e. Pointless! When it's purpose is to link to a blog).

    Once the link has been added inside the heading, you'll need to make the whole card clickable. A common way to do that is by making the card position relative and adding a pseudo element to the link that covers the card with position absolute. Now tye clickable area of the link will cover the card.

    The only other problem I see is the max-width 25% on the tag/category. You need to remove that. It will cause overflow for some users. Let the width of the category/tag be defined by its content and padding only. There's no need for a width or max-width.

    Lastly, there's no need for a media query in this challenge. You can remove it all. Leave the font sizes as the default and don't worry about changing anything.

    0
  • Lokesh•120
    @Lokesh8055Submitted 14 days ago
    What are you most proud of, and what would you do differently next time?
    • Proud of Learning about frontend mentor rules for creating a project.
    What challenges did you encounter, and how did you overcome them?
    • Encountered challenge with Readme file, it's a very good learning for me to create proper documentation
    What specific areas of your project would you like help with?
    • Please go through my project and let me know on the areas that I can improve.

    Qr code component challenge using HTML and CSS

    2
    Grace•31,250
    @grace-snowPosted about 8 hours ago

    Great feedback above and we'll done for applying changes so quickly.

    I've got a few more recommendations:

    • get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
    • Look up how and when to write alt text on images. There are some great posts about this in the resources channel on discord. Remember, alt is human-readable content not code. This image is really important, so needs a proper description. It needs to say what the image is (QR code) and where it goes to in this case (to FrontendMentor.io).
    • when building single components like this you still need to consider the context of where that component would be used in a real site. This card would never be used to serve the main heading on a page, so you know it shouldn't have a h1. Use a lower importance heading level like h2 instead.
    • it's better for performance to link fonts in the html head instead of css imports.
    • make sure the component can't hit screen edges by giving a wrapping element a little padding on all sides (eg body or main in this)

    Note the deployed link seems to be broken above so I can't preview the finished version at all.

    Marked as helpful
    0
  • Dave Quah•660
    @MilleusSubmitted 9 days ago
    What are you most proud of, and what would you do differently next time?

    I'm proud of getting back into Frontend Mentor challenges as I've been away for many months. I aim to dedicate more time into learning outside of work.

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

    The challenge I've encountered is on how to use more meaningful semantics for the element.

    Referencing the MDN website, I've learnt that I could use the datetime attribute of a element to describe the publication date and time of an element.

    Blog preview card using variable fonts

    1
    Grace•31,250
    @grace-snowPosted 8 days ago

    You need to switch that aria-desciribedby to aria-labelledby. Using description means the blog heading is read out after everything repeatedly.

    The other small issues I noted

    • this card would never be used to serve the main heading on a page. So you know it shouldn't have a h1. Use a lower importance heading level like h2 instead.
    • you don't need aria hidden on decorative images. You have correctly left the alt empty so it's already been hidden in the right way.
    • the description on the author image is only repeating the text that's next to it, leading to unnecessary screen reader announcement. It's bringing no value so treat that image as decorative too with an empty alt.
    • font size should always be in rem not px. See https://fedmentor.dev/posts/font-size-px/
    • I strongly recommend you stop using all these complex direct child css selectors. On a big project that will create css with high specifity that's very difficult to maintain. Keep css specificity as low as possible. Use single class selectors.
    • note when links open in a new tab like in that attribution link it's best practice to let screen reader users know in advance. E.g. include some visually hidden text in a span that says "(opens in a new tab)".
    • the design indicates that this whole card should be clickable. See if you can work out how to do that in a simple way without moving the link (it's in the correct place in the html at the moment - I could say exactly how but you may learn better trying to sort this one on your own first, maybe you just forgot to implement etc)
    Marked as helpful
    0
  • Grace•31,250
    @grace-snowPosted 8 days ago

    I'm afraid there are some small but severe accessibility problems in this solution that need fixing.

    1. Every input must be labelled.
    2. You must not use the auto focus attribute. Let people have full control over where focus goes. Currently it's like screen reader and keyboard users will get hijacked.
    3. All of these inputs must have autocomplete attributes set to appropriate values.
    4. The submit button should be aria-desciribedby the unique ID of the terms and conditions paragraph.
    5. The errors need to be announced by screen readers when they appear and should be programmatically linked to their inputs. As you're already conditionally rendering them, the easiest way to address all this is to wrap each error in an extra element like a div that's got a unique ID and an aria-live attribute. Then add aria-desciribedby to the input referencing the id of the error wrapper. This will mean that the errors are announced when they first appear, and on focus of the input (read out after the input label).
    6. You need to remove the max-width 100vw and overflow-x hidden from the html and body. Firstly, these are unnecessary. The html doesn't need styling and should be left alone. The body is already full width by default. But adding the max width can make it impossible for people to read content if they have a large text size or use zoom and view on a smaller screen. People need to be allowed to scroll sideways if overflow ever happens.
    7. Font size needs to be in rem not px. For more info see https://fedmentor.dev/posts/font-size-px/

    And general observation/recommendation:

    • use the fonts and sizes as defined in the style guide (except with font size converted to rem before use)
    • try and get this much closer to the design in terms of these things. Yours is currently much smaller for example.
    • don't forget to adjust line height. Usually that will be unitless like 1. Etc.
    • you seem to be using percentages for various properties and setting widths unnecessarily in lots of places. To limit width, use max-width in rem. Remember padding — that's usually in px on the sides as that's not a property you'd want to scale with the users device width or font size. Using percentages will often end up with confusing or unpredictable results, especially for properties like margin. There is a post about padding vs margin and another about media queries and responsiveness on my site shared above and those articles may help you overall.
    • The other recommendation is to watch Andy Bell's talk "Be the browser's mentor not it's micromanager" as this may help you with overall layout approach.
    0
  • Grace•31,250
    @grace-snowPosted 8 days ago

    Hi,

    This looks good but there are a few issues that should be addressed.

    • the 3 stats need to be an unordered list with 3 list items. Each number and/or word in those can be in spans set to display block to make them stack vertically on each line.
    • the card must not have a height. That's very potentially dangerous for overflow bugs. You can't control the height — you have no control over the end font family, language, font size or line height the end user may have, and all of those could change where lines break and how tall the component needs to be. It's the browsers job to decide the height based off the content, plus any padding, margin or gaps used inside it. You could use min-height but even that is unnecessary.
    • the text half of the card should have padding on all sides. The low level child elements like headings or paragraphs shouldn't need percentage widths. Don't micro-manage. All the component needs for width is a max-width in rem (different on mobile default and larger screens in the media query).
    • it's unnecessary to load the css reset as a whole other file. That impacts performance. Instead, just place those css declarations at the start of the main styles.
    0
  • justine1607•110
    @justine1607Submitted 10 days ago
    What are you most proud of, and what would you do differently next time?

    I am proud that i can increment, decrement and remove item form the cart using vanilla js.

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

    the challenge that i encountered were a lot just let me state one. -honestly it was so difficult because i still lack of knowledge on javascript especially on the removing the item, it took me 2 days before i gets on how the logic will work, but then, because of my enthusiast i have overcome this difficulties by just trying and trying until the code works but also with the understanding on how they work.

    interactive add to cart

    2
    Grace•31,250
    @grace-snowPosted 9 days ago

    I think you're jumping ahead with this site. Particularly in terms of accessible HTML and JS. This is inaccessible to people who use a keyboard or screen reader at the moment, which is a baseline requirement.

    Also the performance is suffering because of the way you're loading the images.

    For example you need to

    • update all image alts especially iconography
    • use interactive elements for all controls
    • ensure those interactive elements always have an accessible name that says what they do (through img alt, sr-only text or aria-label)
    • the cart button wouldnt usually open a modal on click, there would be a form submit. But any button that opens a modal dialog would have aria-haspopup="dialog".
    • use the native html dialog element for the confirm. Follow the docs for that element and it should be accessible by default. Without that, you'll need to do loads of extra work.
    • use the picture element for the product images.
    • give product images an aspect-ratio in css to help loading performance, or a width and height attribute in the markup.
    • probably give them loading="lazy" (although I'm not sure if that works when loading the markup with js... Worth a try! Watching the network tab as you scroll down on a short viewport would show if lazy loading is working).
    • as a general rule, use JS to toggle classes and attributes instead of lots of inline styling. This leads to a much more manageable project. For example, the JS can toggle the hidden attribute instead of display none, OR toggle classes like "is-visible".
    • this may need some sr-only text elements and/or aria-live regions to ensure that screen readers are notified of content changes.
    0
  • justine1607•110
    @justine1607Submitted 10 days ago
    What are you most proud of, and what would you do differently next time?

    I am proud that i can increment, decrement and remove item form the cart using vanilla js.

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

    the challenge that i encountered were a lot just let me state one. -honestly it was so difficult because i still lack of knowledge on javascript especially on the removing the item, it took me 2 days before i gets on how the logic will work, but then, because of my enthusiast i have overcome this difficulties by just trying and trying until the code works but also with the understanding on how they work.

    interactive add to cart

    2
    Grace•31,250
    @grace-snowPosted 9 days ago

    General important point about repositories - node modules must never be pushed to the remote repo!

    To remove

    1. locally delete node modules
    2. Add it to the gitignore (it should have been in there at the start TBH)
    3. Commit and push that change.
    4. Re install locally (npm install)
    5. If working on a repo with a build step you'd give the host (eg Netlify or Vercel) that build command so it can build your site before any deploy. (If all you're doing like here is building some sass on a practice project you can choose to build the css locally before pushing to the remote repo — it's not strictly necessary for the host to build the css for you although that is recommended).
    0
  • Grace•31,250
    @grace-snowPosted 10 days ago

    I've already left a lot of feedback on your blog card solution so won't repeat things here.

    But this challenge also needs changes. The whole questions should be inside the buttons, not just the icons.

    This is an accordion challenge, which means it is a set of disclosures. These have very specific accessibility requirements that you must follow. See the article here for essential changes needed https://fedmentor.dev/posts/disclosure-ui/

    Once you've fixed the markup you should change the JS too. Loop over all the buttons and add event listeners to run a function. That function should change the aria expanded attribute and optionally a class or data attribute to manage the visibility of the answer. The same single function should run on click of any of the buttons. Remove all onclicks in the html and rely on the one short js function instead.

    Marked as helpful
    0
  • Grace•31,250
    @grace-snowPosted 10 days ago

    I'm afraid this is missing essential html and css foundations. It's important to get feedback before moving on to the next challenge as I can see you are repeating the same mistakes each time.

    • all content should be contained within landmarks. This card should sit within a main landmark.
    • Indent your code consistently so it's easier to read and spot bugs. Your code editor can even do this formatting automatically for you with prettier.
    • the image doesn't need wrapping in an extra div. Keep the html structure simple, only adding extra elements when it's necessary for meaning or layout.
    • the img element must always have an alt attribute. Make sure you understand how and when to write a description and when it should be blank. Look up Craig Abbott blog article about alt text, it should help.
    • this card has a heading. You must use heading elements when they are appropriate.
    • you've missed the essential function of this card. The design clearly shows that it's meant to be clickable. It's purpose is to signpost (link) people to a blog post. Yet you've not included a link in here! There needs to be a link inside the heading, wrapping the blog title text.
    • once that's been added, you need to make the whole card clickable. To do that you make the card position relative and add an absolutely positioned pseudo element to the link that will cover the card.
    • I would expect the author image and name to be inside a div set to display grid or flex to get the alignment right.
    • never limit the height of elements that contain text, including the body. That's extremely important. The body in this needs a min-height 100svh (or vh) for the centering and that's all in terms of height. Note the difference between min-height and height. One is flexible the other is not.
    • exactly the same principle: the card must not have an explicit height either. But this doesn't even need a min-height. Let the browser do it's job and decide what height is needed based on the content inside.
    • similarly, the card doesn't need a width either. Giving elements fixed widths like this is not a robust way to approach layouts. Instead, give the card a max-width in rem. Note the difference between max width and width. Max width means the card can go narrower when it needs to like on small screens.
    • get into the habit of including a full modern css reset at the start of the styles in every project. The image doesn't need anything for its sizing except what is already included in the css reset — display block and max-width 100%. Look up Andy Bell's css reset, or Josh c Comeau's one. Read about them and use one of them.
    • font size must be in rem not px. See https://fedmentor.dev/posts/font-size-px/
    • the card should have padding on all sides (usually in px as this is a property you probably don't want to scale with the users font size). The children inside the card only need vertical margins. They shouldn't have side margins. If you're confused about when to use padding and margin there is a blog post at the same site I've shared above.

    Make sure you apply all these learnings to this challenge and all of the others you've done.

    Marked as helpful
    0
  • Grace•31,250
    @grace-snowPosted 11 days ago

    Hi, I don't think this looks enough like the design yet. And you're missing a few key foundations in the html.

    • all content should be contained within landmarks. Everything in this design should be in a main landmark. The attribution should be in a footer.
    • look up how and when to write alt text on images. An image's alternative description shouldn't ever include words like "image" because it's already an image role. The description needs to provide the same value that the image itself provides.
    • you don't need all these extra divs. The whole component is one div, all text content should be in another (a child of the first) and the last div is for the preparation section because that has a different background colour. You don't need any others. Keep it simple. All these extra divs do is make you need more work in the styling.
    • strong is the tag for bold emphasis, not b
    • headings should stay in hierarchical order. So preparation is a h2 not h3.
    • in the table, it's essential to use header cells (th) for header cell content. These should also have a scope="row" to make it clear they are row headers.
    • the p Google font preconnect links must be before the font links. Order matters in the html head!
    • this should only have one css file, definitely not two.
    • make sure you update your attribution link to your frontend mentor profile page or github profile page.
    • get into the habit of including a full modern css reset at the start of the styles in every project you do. Look up Andy Bell's or Josh Comeau's —read and understand it then use one of those.
    • font size must never be in px. Use rem. https://fedmentor.dev/posts/font-size-px/.
    • mobile styles should be the default. This is totally broken on mobile. Pay attention to it! There is an article on the same FEDmentor.dev site about how to approach responsive design and media queries which may help you.
    • this component needs a max width in rem.
    Marked as helpful
    0
  • Bamo-D-Abdallah•300
    @Bamo-D-AbdallahSubmitted 12 days ago
    What specific areas of your project would you like help with?

    Overall code feedback especially the organization of code or any bad practice that I might've overlooked?

    3 Column Preview Card Component

    1
    Grace•31,250
    @grace-snowPosted 12 days ago

    This looks pretty good but I spot a few issues.

    • there is no h1 in this design. All headings should be at the same level (h2). Sedans is not the heading for the whole page / all other content so shouldn't have a h1.
    • it's important when link text repeats like this that the accessible name is unique to the context where those links are placed in the content. The easiest way to do that would be adding the car type into the links insode a visually-hidden (sr-only) span.
    • in the large screen view the paragraphs should have margin-bottom auto. This is so that the buttons are always aligned at the bottom of the cards, even if one card has a longer paragraph.
    • make sure you add a little padding to the body or main on this so content can't hit the screen edges.
    Marked as helpful
    2
  • frost3dWave•190
    @frost3dWaveSubmitted 12 days ago
    What challenges did you encounter, and how did you overcome them?

    not much.

    What specific areas of your project would you like help with?
    • responsiveness of site & fluidness of typography and anything in between related to them.
    • As always if you see something i may be doing wrong or if there is a better way to achieve something please let me know!

    Four card feature section

    1
    Grace•31,250
    @grace-snowPosted 12 days ago

    So this looks great, but there's one problem that stood out straight away to me in this – This design does not include a header. It's important to understand what page-level landmarks are each for. The header is a banner role, which means it is meant to hold primary content that repeats over multiple pages or a whole site. There should be no page-specific content in a header, definitely not a h1.

    You don't need the br element inside the main heading. Note some screen readers will announce br as "break" which isn't needed here. Instead, just set one of the halves of the heading to display block.

    The icons in this are decorative so must have empty alt. It's great that you're trying to write good alt text on images, but in this case the descriptions are definitely unnecessary and far too verbose.

    ⚠️ This is important - The cards must not have a height or max-height. That is going to cause overflow problems for some users. There's no reason for these cards to have a max width or height actually. Let the grid template define the sizes of the children. That's an important principle when working with grid. Let the grid have control.

    This really only needs to be grid by default, then have the max-width and columns of that grid changed for larger screens. You're making the css longer unnecessarily by using flex on mobile and adding an extra media query.

    Placing the max width on the grid itself is more usual on any site where you will have a max content width.

    Marked as helpful
    3