Skip to content
OverviewSolutions
10
Comments
23
P
Jairo Valderrama
@jairovg

All comments

  • Jairo Valderrama•420
    @jairovgPosted about 1 year ago

    Hi Carlos, congrats on your solution; it really pleases me to see implementations using what was discussed in our mentorship sessions; here are some comments that might help you to improve your solution:

    Accessibility and semantics

    • You may enhance the form experience adding some attributes to the input element like: aria-required so not just the form has a native validation status but any assistive technology to be able to correctly mention it; and aria-describedby so assistive technology would be able to give a better context on the input when the user has an error on it, and finally an aria-invalid="true" when it has an error state.
    • The span.warning-msg may have an aria-live="assertive" attribute so the assistive technologies may read out loud any error message.
    • Regarding your ul.logos-list element; it may be wrapped in a nav to enhance its semantics and also, each link should have a descriptive text, it's not enough with the alternative text in the images.
    • There is also an accessibility issue related to the same element, as the anchors do not have any other state but their default one. Any user who tries to navigate on the page using the keyboard may get lost once it sets the focus on any of these elements.

    Styles

    • You may want to remove the focus state in the input element and instead handle a focus-within in the form, in order to remove the odd rounded outline present in the input in tablets and desktop devices.
    • Your request access button misses its focusstate.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    Marked as helpful
    0
  • Jairo Valderrama•420
    @jairovgPosted about 1 year ago

    Hi Juan, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • You are duplicating nodes to have the same component depending on the device, so you have two articles, one for mobile and one for tablets or desktop ones. Try to create you elements using only one component, no matter the device and avoid duplicating it as it's a bad practice.
    • The email input has a visual accessibility issue, as it does not have any other state but its default one. Any user that tries to navigate on the page may get lost once it sets the focus on this element.
    • Even if you're validating the form before its submission, you may enhance the experience using some attributes like: required and aria-required so not just the form has a native validation status but any assistive technology to be able to correctly mention it; aria-labelledby and aria-describedby so assistive technology would be able to give a better context on the input state when focused or even when the user has an error on it; and an aria-invalid="true" when it has an error state.
    • The span.text-fem-red may have an aria-live="assertive" attribute so the assistive technologies may read out loud any error message.
    • Regarding your nav elements; the anchors should be wrapped in an ul to enhance its semantics and also, each link should have a descriptive text, it's not enough with the alternative text in the images.

    Styles

    • Do not let the browser have its native focus state to provide a unique experience across devices. Each browser may use this outline differently.

    Vue

    • You can modularize your solution by creating smaller components instead of handling one single App component. Some candidates to components may be icon, request-form, and podcasts among others.

    Others

    • You have a prettier config but the project is 1) depending on the developer to install it on his/her machine instead of define a local version in your package.json dev dependencies, 2) there is an improvement here you can make using modules like husky to prevent commits with code that has not been run prettier yet and lint-stage to only run prettier on staged files; this latter module will be helpful on large projects with thousands of modules.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    Marked as helpful
    1
  • PMZ Programmer•30
    @pmzproSubmitted about 1 year ago

    Hello Everyone!! =)

    This is my third challenge. I feel like I'm getting better and understanding more but still have a lot to learn.

    What I Learned in this challenge

    • Learned more how to make the layout responsive for the user experience depending on their device's screen size.
    • Learned how to add a transparent background in <a> tag (button) for the active effect.

    Built with

    • Semantic HTML5 markup
    • CSS custom properties
    • Flexbox
    • CSS Grid

    Feel free to say what I could've done better

    3-Columns Preview Card Component using HTML CSS, CSS GRID AND FLEXBOX

    1
    Jairo Valderrama•420
    @jairovgPosted about 1 year ago

    Hi PMZ, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • This is your heading map:
      Frontend Mentor | 3-column preview card component
    2 Sedans
    2 suvs
    2 luxury

    All headings should have a hierarchy and a meaning. You can check this checklist provided by Deque University to learn some best practices related to headings that will help you improve your structure. You may use a .sr-only class so the missing page title may be visually hidden and help to enhance the assistive technologies experience.

    • There is an axe-core ruleset that should be considered. It's related to the document title, in this case, the best practice that may be applied is Put all unique information first. ...company’s name or brand ... should go after the unique content... So the suggested document title should be 3-column preview card component | Frontend Mentor.
    • There is another tag you may use for your .box element instead of a div to improve its semantics.

    Styles

    • Do not let the browser have its native focus state to provide a unique experience across devices. Each browser may use this outline differently.
    • It's better to use colour tokens based on their semantic use rather than their colour names. It's better for future refactors or a theme feature.
    • I invite you to modularize your styles instead of having all of them in a single file. You may use CSS naming methodologies that may help you with this.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    1
  • Anderson F. Palacio•20
    @Anderson-PalacioSubmitted over 1 year ago

    • What did you find difficult while building the project? breakpoints, the correct use of BEM and semantics

    • Do you have any questions about best practices? What would be the correct way to name similar colors, where the only difference could be saturation or lightness?

    Scss, html, JS

    1
    Jairo Valderrama•420
    @jairovgPosted about 1 year ago

    Hi Anderson, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • This is your heading map:
      Frontend Mentor | Results summary component
    2 Your Result
      3 Great
    2 Summary
    • All headings should have a hierarchy and a meaning. You can check this checklist provided by Deque University to learn some best practices related to headings that will help you improve your structure.
    • All interactive elements, like anchors or buttons should provide different states to be universally accessible; those states should at least be rest (default state), hover, focus and active. Check for the requirements in the provided screenshots and try to not let the browser have the focus one to provide a unique experience across devices.
    • You're creating two section elements. There is an interesting blog post I like to share, named <article> vs. <section>: How To Choose The Right One which I invite you to read to check if they are the correct chosen elements.
    • Your solution has an a11y issue injected by the lack of a main landmark. You can read more about this issue on the Deque University Axe rules page.
    • This project has some opportunities to use techniques like sr-only or some aria attributes to improve the experience of assistive technologies. I invite you to research about it and to improve your solution with it.

    Styles

    • Your solution is missing the shadow in the component container.
    • The button element uses some browser native styles, including its focus status. As mentioned above, having a custom style following the style guides will provide a unique user experience across devices.
    • The component uses px as the unit for properties that must be dependent on the user preferences for font-size. I suggest you use rem or em depending on the use case as the units.* The project has media queries that miss devices (like extra large). If you set your monitor to a resolution greater than 1440px you'll notice the visual issue your project has.
    • You are using media queries to support a specific range of devices and miss some others. Try setting your device to 1441px to see the issue.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    0
  • Huey.io•240
    @huey-ioSubmitted over 1 year ago

    Not my best work, I've been at it for a few days on and off. Life has been quite chaotic lately and every time I seem to open this project in specific I lost where I was last left at XD.

    Problems I faced were : Grid spans, I noticed I need more practice using them in projects, I barely use grid as is lol. Another problem was remembering to build the desktop version while building the mobile version. Saves alot of time, more problems were spacing. I seem to have had quite a bit of overflow between the "3.0 textarea" and the bottom area. Moving forward will be aiming to put 110% into each and every project regardless of tiredness or willingness to code.

    All Advice appreciated -

    FEM Tailwind News

    2
    Jairo Valderrama•420
    @jairovgPosted over 1 year ago

    Hi Huey.io, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • This is your heading map:
    Tailwind News
    
3 - The Bright    
3 - Hydrogen VS Electric Cars    
3 - The Downsides of AI Artistry    
3 - Is VC Funding Drying Up?
    • All headings should have a hierarchy and also there are some missing ones. You can check this checklist provided by Deque University to know some best practices related to headings that will help you to improve your structure.
    • All interactive elements, like anchors or buttons should provide different states to be universally accessible; those states should at least be rest (default state), hover, focus and active. Check for the requirements in the provided screenshots and try to not let the browser use the focus one to provide a unique experience across devices.
    • The solution lacks a lot of semantic elements that may enhance the meaning of your page and will play a key role in assistive technologies like screen readers. A sample is the elements in your section#bottom; the page is listing in an ordered way different elements. Each repetitive element is required to be a link to other pages.
    • You're opting to use decorative images in all sections. Think about if the image really adds a meaning to the content and if that's so, then add a descriptive alt text to it, otherwise, they will be treated as decorative only and will be ignored by assistive technologies like screen readers.
    • The Web Accessibility Checklist by Deque University, will help you to improve your page's semantic structure and enhance its accessibility also.

    Other

    • You're duplicating all the navigation elements to display desktop and mobile versions. It should be created as a single component changing its appearance using CSS only.
    • The page does not have a limit which makes the elements expand to full width. Check on the provided screenshot to know the page max-width.

    Regarding your questions/doubts

    • ...problem was remembering to build the desktop version while building the mobile version.

    As mentioned by Matias, think about components and try to make them look progressively good on mobile first and switch to desktop version; identifying the components first may prevent to inject issues like treating the hero image (image-web-3) as part of the hero component, which should be a section element, and realizing that it should not be above the news component, which should be another section element of the page.

    • Grid spans, I noticed I need more practice using them in projects...

    Same as the above comment, think about components first and do not try to create a grid for all the page elements at once. You'll notice that this approach will allow you to reuse components in following challenges or projects.

    There are also good resources out there that will help you improve your skills using grids, here are some of them:

    • A Complete Guide to CSS Grid.
    • Grid garden.
    • Finally, I suggest you not try going to the next challenge without solving the issues in this challenge first. Give the 110% you mentioned improving this challenge to get to the next one with more skills and new approaches and practices to apply.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    Marked as helpful
    1
  • CARLOS ALZATE•40
    @carlosalzatepSubmitted over 1 year ago

    Which areas of your code are you unsure of?

    Not sure about the H2 tags used for the last section titles. Neither sure about having 2 <ul><li>, one for Desktop and other for Mobile

    Landing page HTML/JS/CSS

    2
    Jairo Valderrama•420
    @jairovgPosted over 1 year ago

    Hi Carlos, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • Here is your heading map:
    1 - The Bright Future of Web 3.0?
2 - New
3 - Hydrogen VS Electric Cars
3 - The Downsides of AI Artistry
3 - Is VC Funding Drying Up?
2 - Reviving Retro PCs
2 - Top 10 Laptops of 2022
2 - The Growth of Gaming
    • Even the New title has a correct hierarchy, this is the text the assistive technology will read out loud; I would consider an enhanced title with a part for screen readers only like "New articles" or "New posts".
    • The three elements of the .top-news should be h3 each of them and the section.top-news element misses its heading to enhance the assistive technologies experience. You may use a .sr-only class so it may be visually hidden.
    • The same section uses an ordered list to display its articles, however, you're using a span to enumerate the items.
    • The images in the same .top-news section are being treated as decorative images as they have an empty alt attribute. I'm wondering if they don't add any value to the related content.
    • I used to recommend the size hack you're using to override the base font size for simple calculations, but not anymore. I invite you to read this interesting blog post named Should I change the default HTML font-size to 62.5%? which explains the cons behind this hack and an approach to calculate the font-size using sass functions.

    Styles

    • It's better to use colour tokens based on their semantic use rather than their colour names. It's better for future refactors or a theme feature, in which case it's even better to use CSS variables.
    • Try to not use deep nesting in your sass rules and not to make them dependent on other elements or it will be difficult to reuse some of your components. Instead, think about them as independent components.
    • The images of the .top-news section are stretching at some breakpoints.

    Others

    • You're using two nav for the .header-nav duplicating all its children. It should be created as a single component changing its appearance using css only.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    Marked as helpful
    0
  • Jairo Valderrama•420
    @jairovgPosted over 1 year ago

    Hi Edward, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • Here is your heading map:
    1 - The Bright Future of Web 3.0?
2 - New
3 - Hydrogen VS Electric Cars
3 - The Downsides of AI Artistry
3 - Is VC Funding Drying Up?
3 - Reviving Retro PCs
3 - Top 10 Laptops of 2022
3 - The Grownth of Gaming
    • Even the New title has a correct hierarchy, this is the text the assistive technology will read out loud; I would consider an enhanced title with a part for screen readers only like "New articles" or "New posts".
    • Notice that you have three elements that even belong to an h3 in its hierarchy, their parent is not correct.
    • The page title and icon are related to Vite and not to the challenge's scope.
    • The section.top element misses its heading to enhance the assistive technologies experience. You may use a .sr-only class so it may be visually hidden.
    • The same section uses an ordered list to display its articles, however, you're using a span to enumerate the items.

    Styles

    • Nice to see colour variables using their semantic use rather than their colour names, however, some of the colour names are used in some elements.

    Others

    • The button.header__menu and button.nav__close elements might be treated as a single toggle element.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    Marked as helpful
    0
  • Jairo Valderrama•420
    @jairovgPosted over 1 year ago

    Hi @josh76543210, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • This is your heading map:
    Frontend Mentor | News homepage

1 - The Bright Future of Web 3.0?
2 - New    
3 - Hydrogen VS Electric Cars    
3 - The Downsides of AI Artistry    
3 - Is VC Funding Drying Up?
2 - 01    
3 - Reviving Retro PCs
2 - 02    
3 - Top 10 Laptops of 2022
2 - 03    
3 - The Growth of Gaming
    • Even the New title has a correct hierarchy, this is the text the assistive technology will read out loud; I would consider an extended title to screen readers only like "New articles".
    • The other h2 seems incorrect to me and they should be the h3 you have as level 2 titles.
    • There is an axe-core ruleset that can be improved. It's related to the document title, in this case, the best practice that may be applied is Put all unique information first. ...company’s name or brand ... should go after the unique content.. So the suggested document title should be News homepage | Frontend Mentor
    • You have multiple elements with a hover state, but they are not links and are not accessible with the keyboard either. So you're injecting an a11y issue. Here is a page from Deque with more info about the keyboard-inaccessible.
    • Nice setup you made for the page responsive considering users that may change their browser font size.
    • You're using the aside to handle the mobile version for the main navigation. You may achieve the same result using one single navigation, avoiding duplicating the elements, which is not a good practice and may impact SEO.

    Styles

    • Try some naming methodologies like BEM. They will allow you to have a better structure in your style rules and also to have better rules specificity.
    • Using colour variables linked to colour names, like grayish-blue or soft-orange may take you to unnecessary refactors if the colour changes or you apply theming to the page; instead, try to use variables scoped to features or categories using tokens.
    • Regarding the hamburger menu, you're using another resource to be downloaded when the user hovers over the element, try it using a mask image changing the element background colour on hover.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    Marked as helpful
    0
  • Jairo Valderrama•420
    @jairovgPosted over 1 year ago

    Hi @andresmdzc, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • You're thinking on the component body as an <article> element and the entire component as a <section>. I invite you to look at this interesting blog post that may help you improve the elements you're choosing.
    • You're right in using an alt attribute for your image but think about a better image description because this is the input an assistive technology may use to read out loud to a user.
    • You have some a11y issues; you can use tools like axe dev tools if you're not using them yet.

    Styles

    • I don't recommend using element styles unless setting an override style or a known pattern to be used across your site. Otherwise, use naming methodologies like BEM, so your styles get modularized.
    • Take a look at different breakpoints. Having % units is correct to define widths, but if you don't check at other breakpoints, you're components will get odd behaviours if you don't set some component limits, which you'll get in your UI designs.
    • You're missing some other styles like rounded corners, shadows, etc.

    I hope you find it useful. I'm happy to look at your solution if you make other changes.

    0
  • Jairo Valderrama•420
    @jairovgPosted over 1 year ago

    Hi @chabandou, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • This is your heading map:
    Frontend Mentor | Clipboard landing page

1 A history of everything you copy
2 Keep track of your snippets
3 Quick Search
3 iCloud Sync
3 Complete History
2 Access Clipboard anywhere
2 Supercharge your workflow
3 Create blacklists
3 Plain text snippets
3 Sneak preview
2 Clipboard for iOS and Mac OS

    This heading map is correct, but there is an axe-core ruleset that can be improved. It's related to the document title, in this case, the best practice that may be applied is Put all unique information first. ...company’s name or brand ... should go after the unique content.. So the suggested document title should be Clipboard landing page | Frontend Mentor

    • The footer links should be created using a navigation landmark. Here is a sample you can check.
    • The social icons should be treated as links.
    • You're not allowing to change the font size to small devices (less than 375px) as you've set a fixed base size of 14px.
    • Even if you're setting an alt text to content images, try to add a real description of them as it's the text that the assistive technology will read out loud.
    • Probably you may enhance the button text for assistive technology also, something like Download Clipboard for iOS.

    Styles

    • Consider adding a cursor pointer for the buttons when the user hovers his/her mouse.

    I hope you find it useful. I'm happy to take another look at your solution if you make some other changes.

    Marked as helpful
    1
  • Oscar Vaquera•270
    @vaqueraoscar0Submitted over 1 year ago

    Technologies Used:

    • React
    • HTML5
    • CSS3

    Challenges and Notes:

    The development of this project was relatively easy, and I didn't encounter any significant issues. However, due to the limitations of the free account on Frontend Mentor, I didn't have access to the proper sizes for the product card. As a result, some adjustments may be needed to ensure the card's dimensions match the design specifications accurately.

    Additionally, since the specific colors for buttons and text were not specified (didn't look accurate), I made assumptions and used default colors in the project.

    Despite these minor challenges, the project progressed smoothly, leveraging React to create reusable components and CSS for styling. Overall, it was a great experience developing this product preview card component.

    React Product Preview Card Component

    1
    Jairo Valderrama•420
    @jairovgPosted over 1 year ago

    Hi @vaqueraoscar0, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • Take a look at how you can improve the component structure using semantic elements. Here is a blog post from freeCodeCamp that may help you.
    • You're setting the image base on some logic done in your JS code; even if it's not wrong, as it gets the goal, you can achieve the same result natively with the <picture> element. Here is the documentation from MDN.
    • Try to use the font sizes using rem units. You'll make the site more accessible to people who need it.
    • Try adding an alt text for the perfume describing the image, not just with product, as this will be the text used by the assistive technology.
    • Think if the cart icon is adding something to your content, if so, leave its alt text, otherwise convert the image to a decorative one or remove its alt text or handling as a background image.
    • This is your heading map:
    React App

1 Gabrielle Essence Eau De Parfum
5 $149.99
5 Add To Cart

    Think about headings as a table of content of your site, they need to have a hierarchy. Here you can read more about heading structure.

    The structure you're using injects an a11y issue: Heading levels should only increase by one. Here you can read more info about this issue.

    Styles

    • Take a look at xs devices, 320px, like iPhone SE. There is an issue with your component at that breakpoint.
    • I noticed you're trying to add a hover effect to the entire card. That's a good idea, but there is an issue with the implementation. With your current code, if you try to add the click handler to your <button>, if the user clicks outside this element, probably it won't take the action unless you add a click handler to the card also. You need to expand the button click zone, and it may be achieved using a pseudo-element.
    • Consider also adding a cursor pointer on hover and when the card has the cursor hover, change the button state to hover also.

    I hope you find it useful. I'm happy to take another look at your solution if you make some other changes.

    0
  • Víctor Andrés Hurtado Gutiérrez•340
    @vianhgSubmitted about 2 years ago

    How would you split the page into Components? This is my "hierarchy"

    Main:
  |-  RatingCard
      |-  Star image
      |-  Card title
      |-  Card body
          |- Text
          |- RatingNumberComponents
             |- five RatingNumberComponent (radio button)
          |- Submit button
      |- ThankComponent
        |- Header
          |- img
          |- badge (message you selected 4 out of 5)
        |- Body
          |- title
          |- description

    Interactive review vite+vue+ts

    1
    Jairo Valderrama•420
    @jairovgPosted over 1 year ago

    Hi @vianhg, congrats on your solution; here are some comments that might help you to improve it:

    Accessibility and semantics

    • Think about if the star image adds some meaning to your content, if not, then it should be considered as a decorative image and its alt attribute should be removed.
    • Same comment for the thanks image.
    • You may wrap all your input and button elements in a form one.
    • The rating elements are not reachable using the keyboard.

    Styles

    • You're misleading some BEM concepts, I've seen some modifiers in your code when they should be elements instead. i.e.: card--body should be card__body.
    • The submit button misses a pointer cursor, this injects an a11y issue.

    Regarding your questions/doubts

    • The components structure is correct but, try to refactor it a bit. Take a look at the structure you have for the RatingCard and ThankComponent; they are sharing the same structure, as both are a Card, so think about how using a single reusable component you may achieve the same result.

    I hope you find it useful. I'm happy to take another look at your solution if you make some other changes.

    Marked as helpful
    1