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

All comments

  • librart•10
    @librartSubmitted 3 days ago
    What are you most proud of, and what would you do differently next time?

    I am very proud because it turns out I was able to complete this challenge even though I was not really sure about the code, especially about responsive design code.

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

    Mostly about the responsive design code.

    I looked at the resources again and tried again.

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

    I think I would love to get feedback for all of my code from HTML structure to CSS styles.

    QR code component using HTML CSS Flex

    2
    Grace•31,530
    @grace-snowPosted 2 days ago

    Hi there,

    Well done on completing your first attempt. There are some common beginner mistakes in here that you should fix and will set you up well for future challenges. I'll try to list them all out and hope it helps.

    • pop the styles in their own file and link it from the html head.
    • all content should be contained within landmarks. That means this QR code component (the card) should sit within a main and the attribution should sit within a footer (outside of the main).
    • the image in this is the most important content on the page. It needs a proper alt description that says what the image is (QR code) and where it goes (to FrontendMentor.io). Craig Abbot has a great blog post about how and when to write alt text on images that you should look up and read.
    • when building single component demos like this we developers still need to consider the context of how they'll be used in a real site. This card would never be used to serve the main heading on a. So you know it shouldn't have a h1. Use a lower importance heading level like h2 instead.
    • just as an aside for learning: Headings matter. They should only be used for heading content and should go in hierarchical order on a page. Think of heading levels like the contents page of a big document: the headings give an outline of the content of the page and that's used by search engines and screen readers to understand the content.
    • make sure you update your own attribution link. It can go to your frontend mentor or github profile page for example.
    • in css its important to always include 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.
    • remove all widths and heights from this challenge. You don't need them anywhere. Never limit the width or height of the body like this. All the body needs is a min-height 100svh (the body is only as tall as it's content by default) and flex column properties to center the content on the page.
    • the card only needs a max width in rem. No width. And no height. It's really important to grasp this principle. Setting a max-width allows the card to be narrower when it needs to be like on a small screen, whereas width is fixed. And you never want to limit the height of elements that contain text. Let the browser do it's job and decide what height is needed based on the content inside. As soon as you set a height that is fixed and the component will break if users change font styles like size or family and if authors change the amount of content in the card.
    • the image doesn't need anything to size it beyond what is already included in a css reset width: 100%; display: block;. If you want you can give this image an aspect-ratio of 1 for a small performance boost.
    • never set font size in pixels. Use rem instead so the text will respect users text size settings. See https://fedmentor.dev/posts/font-size-px/
    • you don't need a media query in this challenge.
    • make sure you fully understand the difference between padding and margin. There's a post about that on the same linked site.

    Good luck.

    Marked as helpful
    0
  • Svetlozar Stoykov•60
    @svetstoykovSubmitted 10 months ago

    Here is my solution to the FAQ accordion project. My biggest struggle here was positioning the background image. Honestly it took a lot of trial and error. Nevertheless it was a fun project. I used React to make the page responsive and also added transition animations.

    FAQ Accordion with React

    #react
    1
    Grace•31,530
    @grace-snowPosted 2 days ago

    Hi, similar to the last I left feedback on, this needs the html rewriting. Accordions like this must be coded as disclosure buttons. You can't have click listeners on divs (try using the solution with a keyboard only and you'll see it doesn't work). You can learn more about disclosures on the APG (ARIA authoring practices guide) website looking at the patterns page or I've done a write up on my website: https://fedmentor.dev/posts/disclosure-ui/.

    All the javascript needs to do in this challenge is toggle the aria-expanded value on some buttons. Everything else can be handled with the styling.

    It's really important to carefully consider what html is appropriate, it's the foundation of everything else.

    Marked as helpful
    2
  • Grace•31,530
    @grace-snowPosted 2 days ago

    It's like you've jumped ahead a bit in this I think. There are a lot of foundational html (accessibility) and css issues that are usually countered before reaching a challenge of this complexity.

    For example

    • it's unusable on mobile because the height has been limited to the screen height.
    • there are missing essential alt attributes on img elements
    • I couldn't see any landmarks glancing through the code.
    • I couldn't see a fieldset and legend unless I missed that somewhere.

    I recommend you focus in on html learning in particular and maybe get feedback on simpler challenges before refactoring this one.

    Marked as helpful
    1
  • Blenimator•20
    @BlenimatorSubmitted 6 days ago
    What are you most proud of, and what would you do differently next time?

    Proud to have it done all by myself so far without any help, except what i found on the web. Also completing/working on the first challenge is a huge step for me already.

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

    Mostly being confused what might be right or wrong. Watching multiple tutorials about similar or the same topic will lead to different ideas or solutions. Finding out which works best or works at all was the trickiest part for me. Surely the code isn't great yet, but it came close to the desired design which i tried to stick to as much as i could.

    So piece by piece it all came together to a somewhat decent looking page overall.

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

    Surely in the area of planning. At first glance it looked like a doable challenge, but in the end it would have been better to try identify the structure of the page directly and plan it accordingly. How do i plan ahead and how do i analyse other websites?

    It feels like i do have lots of unnecessary code in the style.css which could have been solved better/more elegant. How to avoid unnecessary code?

    What confuses me the most is the responsive part in the challenge and how to achieve different looks for desktop/mobile version. How to make sure to deliver the proper design sizes?

    Recipe page with basic html and css usuage

    1
    Grace•31,530
    @grace-snowPosted 4 days ago

    Well done on your first challenge! I'll list out the issues I see but don't be discouraged, it's normal to get so many and these are very common beginner mistakes.

    This challenge is mostly about html so it's important you take time to focus learning there early on. So many people skip past html but it is the foundation to everything else. If you don't grasp how to translate designs into appropriate well structured html solutions will never be accessible. Get familiar with available elements and when to use them, especially buttons, links, unordered ordered and description lists, table cell types and attributes, heading levels, dialog, picture and more. Start to look at designs and think about what html would be suitable.

    • all content should be contained within landmarks. Everything in this design belongs inside a main landmark. You need one on main on every full webpage so it's a good practice to get into the habit of including them even to wrap single component demo challenges. If you include the attribution, that would go in a footer outside of main.
    • the image alt is important content in this design. It needs to convey the same information/value as the image. Craig Abbotts blog has a great post about how and when to write alt text on images.
    • Heading order really matters. There should only be one h1 on this page. All other headings should be h2s. Heading levels create a hierarchy of the content on the page, much like the contents page of a big document.
    • the table should have header cells (th) inside it, not just data cells (td). "calories" for example is header cell content. Because these are row headers not column headers (which is the default) the header cells should also have scope="row". This is all really important. Assistive technologies like screen readers need the correct table structure to be able to understand the content.

    Now onto the CSS points:

    • 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.
    • never change the width of the body. Think of it as the available screen space. It's like the piece of paper everything gets drawn onto. You can draw your picture smaller on a piece of paper without having to fold or rip that paper to force it to be smaller. Leave it alone. You may find the body is too short sometimes in future challenges. That's because although the body is full width by default it is only the height of its content by default. To solve that when you face it, give the body a min-height: 100svh;.
    • does the body need to be a flex container in this? I don't think it does... When you do make the body or another landmark display flex remember to set its direction to column.
    • remove all styles from the html and place them on the body instead. As a general rule, leave the html alone. It has no default styling and doesn't need styling. It's a wrapper for the head and body for the browser that's all.
    • it's not essential, but consider using the em unit for margin-top on prose content like this (headings and paragraphs). I don't think they need any margin-bottom. The reason I suggest em there is because that unit is relative to the font size. So a margin-top of 1em on a h2 will be bigger than a paragraph.
    • note however, setting base styling on element selectors is usually only done at the very start of a project, straight after the css reset. Most of the time you will want to use single class selectors in css.
    • don't style on IDs! That's not what they're for and can cause all sorts of problems. They are really important for accessibility because their role is for referencing between elements. They can make styling a total maintenance nightmare because of high css specificity and you can't reuse them. Use classes.
    • never set font size in px, even once. Use rem. More info: https://fedmentor.dev/posts/font-size-px/

    Other than that, just try to get this closer to the design. The max width on the component should be smaller and in rem, and the main heading should definitely be smaller. There may be other sizing issues too.

    At the moment this overflows my mobile screen on the sides, which will likely be caused by the missing css reset, possibly too large font size and too large max-width. Make sure you test solutions down to 320px wide.

    Marked as helpful
    0
  • @jeancy•200
    @JeancyjrSubmitted 6 days ago
    What are you most proud of, and what would you do differently next time?

    This was a great project to challenge myself to start applying Javascript without tutorial.

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

    I'm just starting with JS, so I only need to keep building, Which I'm doing daily.

    Faq-accordion JS Challenge

    1
    Grace•31,530
    @grace-snowPosted 4 days ago

    Hi

    As good as this looks on the surface, I’m afraid it is inaccessible and needs re-coding. It’s really important to get the HTML right so that components like this are accessible.

    This must follow an accordion (or disclosure) pattern. You can see all requirements for these ui patterns on the aria authoring practices guide. But essentially these must be opened using a button element that has aria-expanded on it correctly set. The icons also must be inside the button and have empty alt as they are decorative. The aria-expanded value is already communicating the expanded/collapsed state of the disclosure.

    Other pointers:

    • it's better for performance to link fonts in the html head instead of css imports.
    • don't forget to always include 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.
    • the max width on the component should be in rem not px so it scales correctly for all users including those who have a different text size setting.
    • you should be coding mobile first and not using min and max media queries like this. Let the mobile styles be the default. Then set a min-width media query to target larger screens only. This should be defined in rem or em not px so that the layout adjusts nicely when people change their default text size. More info: https://fedmentor.dev/posts/responsive-meaning/
    • you don't need to repeat any style properties in a media query if they are unchanged from the default.
    • it would be better for performance to include the purple image in the html in a picture element instead of using background image. Users will currently have to download both images, whereas the picture element would only download the relevant one for their screen size.
    • try not to change inline styles with javascript. That can make sites very difficult to maintain. Keep concerns separate. Use JS to toggle classes or attributes and let css handle all styling. All that's needed in this is the aria-expanded value to change with javascript as you can use that attribute as the style hook in css to show/hide faq answers. Or if you prefer JS can toggle a class or data- attribute as well as aria-expanded and use that as the style hook instead.

    I hope this is helpful info.

    Marked as helpful
    0
  • ortiz-antonio•230
    @ortiz-antonioSubmitted 5 days ago
    What are you most proud of, and what would you do differently next time?

    I practice progressive enhancement.

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

    I don't show the button if the browser doesn't have JavaScript enabled and display the menu by default.

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

    Any feedback it's welcome

    Progresive Enhancement, Typescript, Postcss

    #cube-css#post-css#progressive-enhancement#accessibility
    1
    Grace•31,530
    @grace-snowPosted 5 days ago

    Hi, good solid effort on this. It's a hard challenge, definitely more complex than it first looks because of the different UIs in mobile and desktop. There are quite a few problems though, and it looks very different to the design. Much much larger, for example and no radius on the avatar.

    I wouldn't say the progressive enhancement has worked correctly either. The popup content is visible by default without JS but definitely should not be styled like that. It's overlapping content in some views and really odd to make it look like a popup still. And on mobile view it is covering the card footer, which is very bad. You need to ensure all content looks and functions correctly if offering a no-JS base version.

    Other issues I notice

    • There's no CSS reset which is causing unnecessary additional scroll.
    • The focus order is incorrect. The popup content must come after the button, because you've triggered it. But currently when I open it the next focusable item is in the footer.
    • Personally, I would follow a non-modal dialog pattern for this using the default HTML dialog element instead of what you have here that's following more of a disclosure pattern. See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog
    • The aria-label should start with "Share" not "open". E.g. aria-label="Share toggle".
    • The toggle button has no visible focus outline when open on mobile because it's the same colour as the background. I would also give it a little outline-offset.
    • The ul is NOT a menu role! All of the roles in this need removing. This is a list of links, nothing else. It doesn't need any special treatment at all.
    • The capitalised SHARE text is a heading (h3)
    • The inline SVGs need to be aria-hidden="true" focusable="false".
    • The way you're using paddings and margins on this is rather strange to be honest. The white area should have padding on all sides. The content inside it only needs vertical margins, and gap in the case of the card footer. This has been over-complicated.
    • Did this challenge only come with one main image? I would have expected some art direction with two images in a picture element, but maybe this challenge only has the one image... I would at least expect the mobile version to have a max-height and not an aspect ratio of 1.
    • Don't forget hover styles.
    Marked as helpful
    2
  • Luthfi•110
    @kankaGateSubmitted 16 days ago
    What are you most proud of, and what would you do differently next time?

    my solution is 99% like the design

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

    Cannot grasp the spacing, font line-height, the padding just from plain image. So i recreate the figma file

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

    on responsive solution

    QR Code Card using HTML and CSS

    2
    Grace•31,530
    @grace-snowPosted 6 days ago

    Hi

    There are a few foundational principles missing in this that it's good to correct early on:

    • All content should be contained within landmarks. This should be inside a main and the attribution inside a footer (the footer as a sibling of the main, not a child of it).
    • The image in this challenge is extremely important content. The alt description needs to say what the image is (QR code) and where it goes (to FrontEndMentor.io). Remember, alt on images is human readable content, not code. This post is worth reading: https://www.craigabbott.co.uk/blog/how-to-write-good-alt-text-for-screen-readers/
    • I strongly advise against changing the HTML/root font size to font-size: 62.5%;. I've written up a full post about why this is unnecessary and potentially damaging for accessibility and for teamwork: https://fedmentor.dev/posts/rem-html-font-size-hack/
    • The body never needs a restricted max-width. Think of it as the piece of paper every design goes on, or the screen people view on. It's not your job to limit that to a smaller area, especially not in pixels.
    • Make sure you get into the habit of including a FULL modern CSS reset at the start of the styles in every project you do. Andy Bell and Josh Comeau have good ones with blogs explaining why they've included what they have.
    • Don't use large margins like 20rem auto to try and center a component on the screen. Instead, make the body and/or main a flex column and use flex properties to center. What you have currently would make the component sit way off screen on mobiles, forcing people to scroll for no reason, and make it sit in a strange position on larger screens too. Again, it's not your job to try and handle the screen the end user is using. This is a nice article about centering with CSS: https://moderncss.dev/complete-guide-to-centering-in-css/
    • I don't think you should be using rem units for the padding on the card. Make sure you understand the implications of each unit you use. If using rem and a user changes their default text size, these paddings would also increase, making the content area potentially very narrow on some screens. Instead, consider using pixels. Or you can go a bit fancy and use a clamp() or max() or min() CSS function, but I think that's overkill here to be honest.
    • Note, it's certainly not wrong, but is less common to use rem for line height. Usually it's unitless like 1.5. If you want to go unitless or use % you calculate it by dividing the line height by the font size. E.g. 24px line height and 16px font size is 24/16 = line-height: 1.5.
    • give the boddy a little padding (e.g. in pixels) to make sure this card can never hit the screen edges.
    Marked as helpful
    1
  • Øystein Håberg•11,380
    @Islandstone89Submitted 10 days ago
    What are you most proud of, and what would you do differently next time?

    I'm satisfied using the fetch API! I also used npm to setup and build the project, as well as doing some configuration in package.json.

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

    Positioning the button was somewhat challenging. I tried doing it with Subgrid, but in the end I ended up using absolute positioning.

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

    As always I'm open for any feedback :)

    Advice Generator

    #sass/scss
    2
    Grace•31,530
    @grace-snowPosted 6 days ago

    This looks good, just a few small improvements to suggest:

    1. Instead of trying to set two competing assertive aria-live regions, wrap the whole card in ONE aria-live set to "polite". Because you have two at the moment they overrule each other, meaning only one will be announced. There is no reason why these would need to be assertive either.
    2. The Dice button label needs to begin with the word "dice" so that it is easy to activate for voice control users. E.g. "Dice - Generate new random advice". This is especially important as you've told people to click the dice button in the help text.
    3. It looks like this challenge intends for you to use the picture element for the divider images as different onese are provided for mobile vs desktop.
    Marked as helpful
    1
  • Øystein Håberg•11,380
    @Islandstone89Submitted 7 days ago
    What are you most proud of, and what would you do differently next time?

    Pretty satisified with the result.

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

    It went fairly well I think.

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

    As always, I'm open for suggestions!

    Skilled E-learning Landing Page

    #accessibility#sass/scss
    1
    Grace•31,530
    @grace-snowPosted 6 days ago

    Nice solution!

    A few things I spotted that should be fixed though:

    • The logo image alt says "Logo Skilled, to homepage" which doesn't make sense. It should just say "Skilled" or "Skilled (logo)". It's not a link, it's just an image of a logo and must begin with the text shown / name of site.
    • The hover styles on the buttons are failing minimum contrast ratios. They need to remain readable (4:5 colour contrast ratio).
    • I don't think that hero image is decorative. It looks like important content, especially as it includes some relevant text. Update the alt.
    • The repeating "Get started" links need more context for screen reader users. They would currently fail WCAG 2.2.4 Link purpose (in context), level A, because they don't describe their destination when someone browses links on the page. To fix, add in relevant sr-only (visually hidden) text inside the link. For example: <a href="#">Get started<span class="sr-only"> - Animation</span></a>.
    Marked as helpful
    1
  • Grace•31,530
    @grace-snowPosted 23 days ago

    Oh no I just wrote loads of feedback then accidentally deleted it 😭 😆

    Oh well ill have to summarise quickly, there wasn't much:

    • you can't aria-labelledby an article by itself! That makes no sense and would lead to a bad experience for screen readers. If you really want to label these it should be pointing to the person's name.
    • the quote paragraphs should be in a blockquote element.
    • I'm not sure why you're adding tailwinds reset and then another reset.

    I can't really comment on css more than that as I'm only looking on mobile.

    Make sure you keep learning html even as you progress in the js learning path.

    Marked as helpful
    1
  • Lisa Allen•550
    @CrypticMangoSubmitted 26 days ago
    What specific areas of your project would you like help with?

    Any tips will be helpful! Thanks for taking the time to look at my work :D

    Recipe Page Vanilla HTML and CSS

    2
    Grace•31,530
    @grace-snowPosted 25 days ago

    This is overflowing on the sides when I view on mobile because you’ve used explicit widths.

    I’m a little surprised to see the code in this actually as it looks like you’ve done a lot of challenges already and can’t have had good quality feedback. I’ll try to list out a load of things and hopefully they can help you go back and refactor this and others.

    • to achieve the layout easily in this I’d expect all text content to be in one div, a sibling to the img. This allows you to make the padding changes in one go from mobile to desktop, applying padding to the text div in mobile only. This removes all the need for strange side paddings or margins on child elements which are unnecessary.
    • aria-label does not belong on an img and must be removed. The alt needs to give a proper description of the image, conveying the same information as the image does. Craig Abbott has written a great blog article about how and when to write alt text if you want to look that up.
    • headings should go in hierarchical order. It’s about structure, not sizes in the design. That means preparation should be a h2.
    • the spans with title class should be strong tags because they are there for emphasis.
    • in the table it is essential to use header and data cells appropriately. They cannot all be header cells.
    • the header cells that should be there should also have the scope attribute set to “row” to make clear they are row headers and not column headers.
    • on the GitHub link at the end the aria-label needs to be placed on the link itself and not on the icon. Meaningless elements cannot have aria-labels, that can only go on an element that has an allowed role.
    • get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell has one that would work well on this and most projects.
    • don’t style the html. The background colour should be on the body. And make sure it’s the same background colour that was in the design.
    • font sizes should always be in rem not px. https://fedmentor.dev/posts/font-size-px/
    • it’s very unusual to place padding on text nodes like headings. Make sure you understand the difference between padding and margin (there’s a post about that on the same site just linked)
    • the component should not have a width, only a max width. And this should be in rem so that the layout scales well for all users including those with a different font size setting. It’s important to understand why you should be using max-width and not width here — so that the component is able to shrink narrower when it needs to like when there isn’t enough space.
    • similarly the img must not have a width. It doesn’t need a rem max width either. It only needs what is already included in any modern css reset: width 100% and display block.
    • the table should be like the image and only have a max-width 100%. The table will also need collapsed borders.
    • really this and most components should be styled mobile first. It wouldn’t be much work to change that even at this stage.
    • I recommend using rem or em to define media queries, not pixels. Again this is to help those who change their default text size, ensuring the layout switches for them at an appropriate time. (There’s an article about media queries on the site too)
    • very little needs to change between the mobile and desktop view in this challenge. If you go through the above changes particularly removing all the side paddings and margins I’d expect there only to be a couple of lines needed in the media query to change the max width of the component and where the padding is applied.

    Good luck.

    Marked as helpful
    3
  • Grace•31,530
    @grace-snowPosted 25 days ago

    This doesn’t really look close enough to the design yet. Try and get the font styles and all styling much closer.

    In the html I notice some important changes are needed:

    • headings must go in hierarchical order and not skip levels. This is a valuable accessibility consideration.
    • bold text for emphasis is done with strong not spans.
    • the table must use header cells for header content.
    • those header cells should also have a scope attribute to make clear they are row headers not column headers.

    And in css

    • get into the habit of including a full modern css reset at the start of the styles in every project you do. Andy Bell or Josh Comeau both have good ones you could use.
    • place font-face declarations before where you use them in the css.
    • make sure you include fallback fonts
    • I’m not sure you mean to be using rem for so many inline paddings and margins. Make sure you’re only using that for properties that you’d want to scale with the user's font size.
    • similarly, you’re using em units a lot which is unexpected and could lead to unwanted results. Only use em on specific properties where you want it to scale with the current element’s font size. For example, button padding is commonly done with em or vertical margins above headings and paragraphs. It’s usually only used quite rarely.
    • the max width on the main component should definitely be in rem. That’s a width that needs to scale with the users font size.
    • Personally I would style the white box as a component inside the main landmark instead of making it be the main landmark. This is because on a real site there may be pages where the main wouldn’t have a limited width and style like this.
    • there needs to be a media query in this challenge. Currently it looks broken viewed on my mobile. Everything is aligned over to the side and the table lines overflow the main landmark. Make sure you check all screen sizes as well as changing browser font size and zooming in/out to cover everything.
    • the line heights look too large on the main heading and there are inconsistencies in font size.
    Marked as helpful
    2