Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • @asbhogal

    Posted

    Hi Emanuel,

    Great work! The design matches the mockups well, your logic is clean and it's great to see grid used for this. Just a few pointers I wanted to mention:

    • Always have your font-size in rem for accessibility purposes. This article by freeCodeCamp explains it in detail Link
    • Locally host your Google Fonts for privacy and performance reasons. Here's a good practical video showing how to do this Link
    • At viewport widths <= 330px, the parent container is touching the browser boundary. You could add some padding or margin just to give it some white/breathing space

    Hope this helps!

    1
  • @GMuhaideb

    Submitted

    I found it difficult to push the code to github using VS code terminal I continuously faced "error: failed to push some refs to 'https://github.com/GMuhaideb/qr-code-component.git'". However, I upload it traditional way using drag and drop the files.

    @asbhogal

    Posted

    Hi Ghadah,

    This is great work, well done! I'd definitely look into the Git terminal issues as soon as you can to avoid any problems with workflows in future projects (unless this is a localized incident.)

    • You have a typo in your text - "bulding"
    • You're missing an alt attribute value, which is important for screen readers and other assistive technologies to be able to understand the purpose and context of the img
    • Your container should ideally be placed inside a <main> element for accessibility purposes as well - it helps direct users to the area of the most prominent content of the document body
    • Always locally host your Google Fonts for privacy and performance reasons. Here's a good video by Kevin Powell which goes through the process of doing this Link
    • You can replace the height with a min-height: 100vh to prevent any vertical overflow

    Hope this helps!

    Marked as helpful

    0
  • Andrija 510

    @andrijaivkovic

    Submitted

    Hello! This is my solution to this challenge. Everything was implemented as shown in the challenge as well as the optional theme switcher. Hope you will find my solution useful and if you have any suggestions or criticism feel free to write about them!

    Rest Countries API App

    #bem#react#react-router#webpack#sass/scss

    1

    @asbhogal

    Posted

    Hi Andrija,

    This is brilliant work - well done! The design matches the mockups really well and its great to see context used for global native state management. Just a few things I've noticed, more from a back-end and maintenance perspective:

    • Avoid using CRA for future projects, as Meta no longer supports this. For client-side applications such as these, Vite is your next best bet, it's relatively low-config and compiles SASS out-of-the-box (you'll just need to install it as a dev dependency as normal)
    • Always locally host your Google Fonts. You can install the typeface you need via the Fontsource dependency, then import the font weights required and declare the relevant properties needed. Here's a link to documentation for the font used here Link
    • Your _Country.scss file is empty. Presumably your styles are referenced elsewhere, in which case this can be removed
    • Netlify throws a 404 error on a page refresh/re-directs (this is a very common problem with React routing, I had the same issue with Vercel.) I've added some documentation here which will hopefully resolve this on your end Link

    Hope this helps!

    Marked as helpful

    1
  • @VickyAzola

    Submitted

    Hi 👋

    Please try my solution, it should show an error if you try to submit the form empty or if your email is invalid. I also added an alert box in case the email is valid.

    Any feedback is welcomed, thanks 🤞

    @asbhogal

    Posted

    Hi Victoria,

    This is great work, well done! The design matches the mockups nicely and the validation seems to function well too. Just a few things I've noticed which are worth considering when refactoring:

    • It's best to use npm packages for your Tailwind instead of a CDN, as this isn't optimized for production (the CDN is designed for in-browser development using online sandboxing tools.) Using the npm version means your build is optimized, compressed and minified and you can also customize your configuration (more on this below) Here's a link to some documentation on how to get this up and running Link I recommend using Vite to handle the compiling of the Tailwind utility classes into standard css. If you're unfamiliar with this, I've also added a link here on how to handle this. Link
    • You should locally host your Google Fonts. With npm environments, you can install the typeface through the Fontsource dependency, then import the weights you need and reference them in your tailwind.config.js file (either by extending or overriding the theme.) I've added some links below regarding this
    • Your image is compressed between 1000px and 768px, it might be worth reviewing and changing the breakpoint classes for when the content flows vertically
    • I can see you've referenced both the images for mobile and desktop, however I can't confirm whether they change respective to the viewpoint. You should ideally have these within a <picture> tag which achieves this by specifying the width for the mobile version, for e.g., to display. Here's a link to an MDN article explaining this in further detail Link

    Hope this helps!

    Marked as helpful

    0
  • @asbhogal

    Posted

    Hi Fatoh,

    This is a good solution - well done! I have noticed a few things however I wanted to highlight below when refactoring:

    • The page isn't fully responsive unfortunately - between 980px and 401px you have compression of the image and overflow of the right-hand side content. I'd suggest reviewing this using your dev tools responsive device viewer and changing the direction/layout for these widths
    • Following on from this, avoid setting fixed width values to child or parent elements (either explicit, or as %) as these cause layout and spacing issues and force content to adapt in not ideal situations. You should have a fixed max-width value (in rem) and a width: 100% on your parent container and let the elements in them occupy the space they need naturally
    • You also need to take your <header> outside of your <main> as this is semantically incorrect
    • Your font-size should always be in rem for accessibility reasons. Here's a good article by freeCodeCamp explaining this in further detail Link
    • I wouldn't say it's necessary to have an absolute positioning for the social icons. Since this is in a footer, you could use either flex or grid to align these to the right and they will respond much better as the viewport width changes
    • The icons, used here as Font Awesome classes, should be inside <a> tags as they link to an external site, preferably also within individual <li> elements as part of an unordered list (<ul>)
    • The images should ideally be managed in the <picture> element in your markup instead of using CSS, with paths to the separate files for desktop and mobile and a viewport width of when the respective ones should appear. Here's a link from MDN explaining this in further detail Link
    • Always locally host your Google Fonts for privacy and performance reasons. Here's a good video showing how to do this Link
    • Also, I couldn't see your stylesheet in your GitHub code (I could only see this through the browser console) - may just want to check this on your end so other developers can look through it easier

    Hope this helps!

    Marked as helpful

    1
  • Andrey 180

    @ANDyouNo

    Submitted

    Hello everyone, Here is YouNo. It's my first splash into SCSS. Please feel free to share any suggestions or comments you may have. <3

    @asbhogal

    Posted

    Hi there,

    Good work! The designs match the mockup nicely. Just a few things I thought worth mentioning:

    • You might want to review your JS code. When I enter in incomplete, invalid or correct email addresses, the page re-renders, so as a user it's unclear which are submitted (only the last should be) and the error messages don't show.
    • Also, try and keep your variables descriptive enough to understand - avoid condensing them to inp and errMsg as well as typos (succesCard) as it can become difficult for developers to debug and identify their purpose especially as code scales
    • It's best to have your font-size as rem values for accessibility purposes. Here's a good link from freeCodeCamp explaining this in further detail Link
    • Locally host your Google Fonts for privacy and performance. You can download them from the website, convert them to woff2 format then upload and reference them in your stylesheets. This video by Kevin Powell explains this process in further detail Link
    • Avoid setting an explicit width value to parent containers as these can cause overflow issues with viewport width changes. It's best to have a max-width with a specified value and a width: 100% so the content either occupies the full available space on viewports less than the max-width or the maximum it can based on the max-width value
    • From a maintenance perspective, split your stylesheet into separate partials for easier location and reduced cognitive load (e.g. having a variables.scss, a responsive.scss, a typography.scss etc.)

    Hope this helps!

    Marked as helpful

    1
  • @NawalMalki

    Submitted

    Hi there 👋, I’m Nawal and this is my solution for this challenge.

    Note that it's only responsive at (max-width: 375px)

    Any suggestions on how I can improve and reduce unnecessary code are welcome!

    @asbhogal

    Posted

    Hi Nawal,

    This is a good solution, well done! Just noted a few things below:

    • Avoid using var when declaring variables. It's best to use let for those which are redeclared later, or const for those that aren't. In this case, you can use const.
    • Variable names should also be in camel case (meaning, the first letter is always lowercase and the first letter of the next word in it is uppercase, e.g. first and firstNumber) and here they should be more clearer (to help other developers identify exactly their purpose, which becomes important especially as your code scales, e.g. instead of Icon, use hamburgerToggle and instead of Show use mobileNav)
    • It's better to let CSS handle the visibility and styling of elements rather than manipulate them in JS (for performance reasons). You can refactor the code to introduce a specific class to display the element as a block and the use the toggle() function in JS to show this via .classList.toggle() on the icon event listener (ie. hamburgerToggle.addEventListener("click", function () { mobileNav.classList.toggle("show"); });
    • It's great you've locally hosted your font, however this should be in woff2 format as this is designed for the web. Here's a link from W3 explaining in further detail Link

    Hope this helps!

    Marked as helpful

    0
  • @asbhogal

    Posted

    Hi there,

    This is a really good first solution! The design matches the mockup nicely - well done! It's also great to see you've locally hosted your fonts too (and in the correct format.) I've listed a few points below worth considering when refactoring this

    • To make this responsive, you can use either flex or grid. I can see you've used flex on the child elements, so it's best in this case to also add this property onto the parent container and adding a flex-direction: column on a width of, for e.g. 640px (using @media queries.) By default, the flex-direction is row, so this doesn't need to be explicitly set.
    • It's also best to avoid setting explicit height and width properties on your parent containers, as this can cause overflow issues as the viewport width changes. Instead, use an explicit max-width value, and a width: 100%, so at any viewport width below the max-width, the content takes up the full amount and at any viewport width above the max-width, the content remains fixed within it. Flex will then take care of adjusting the elements within the space provided
    • For semantic purposes, wrap your parent container in a main element for accessibility purposes - it helps screen readers and assistive technologies identify where the primary content of the document body is located
    • Make sure to have your font-size in rem for accessibility purposes. Here's a good link explaining this in further detail Link

    Hope this helps!

    1
  • @asbhogal

    Posted

    Hi Oscar,

    This is great work, well done! The design matches the mockup well. Just a few things I've noticed which are worth considering for refactoring:

    • It's best to use the picture element to handle responsive images, instead of the CSS background-image property. Here's a good article from MDN explaining how to do this Link
    • You can simplify your code by removing the section container, applying these properties to the main and just having the two divs inside this instead (since it's good practice to have both the max-width and width properties on a parent container)
    • Always locally host your Google Fonts for privacy and performance reasons. Here's a good video by Kevin Powell which shows how to do this Link
    • At 320px your main container touches the viewport boundary. It would be worth adding a padding or margin to the sides just to give them some spacing

    Hope this helps!

    0
  • Jose 80

    @JooseMM

    Submitted

    Hi! The only issue for me was that in Firefox the fetch method was pulling data out of the cache memory instead of the URL provided as an argument. I fixed it specifying the cache policy as it's mention in this article.

    https://hacks.mozilla.org/2016/03/referrer-and-cache-control-apis-for-fetch/

    @asbhogal

    Posted

    Hi Jose,

    Great work! The design matches the mockup nicely and the app is responsive and functions well (I like your coding practices here too with the custom hook and useEffect implementation. Nice.) Just a few things which are worth considering:

    • Always locally host your Google Fonts for privacy and performance reasons. Since you're in an npm environment, you can install the typeface as a Fontsource dependency, import the weights require then declare the relevant properties in your styling. Here's the documentation for the Manrope font Link
    • Avoid using Create React App, as Meta no longer supports this. For client-side applications like these, switch to Vite.

    Hope this helps!

    Marked as helpful

    1
  • @kanishkasubash

    Submitted

    Hi! 👋, Frontend Friends.

    I've just completed my first front-end coding challenge. This is my solution for the QR Code Component.

    🛠️ Built With:

    • Semantic HTML5 markup
    • SASS/SCSS
    • Node.JS
    • GULP.JS
    • Mobile First Approach

    📦Features:

    • Responsiveness (Mobile/Tab/Laptop/Desktop).

    🔖What I am learned:

    • Level Up SCSS
    • CSS Flexbox & Grid. Link

    ❓Questions for the community:

    • I have a question about best practices. Is the Flexbox best layout module to design flexible responsive layout structure?

    💡Any suggestions on how I can improve are welcome!

    😊I'll be happy to hear any feedback and advice! Thank you.

    @asbhogal

    Posted

    Hi Kanishka,

    Great work! In answer to your question, flexbox is generally the best modern layout method to use for flexible responsive structures. Grid would be used for content where you want better control of their layout using columns and rows. This article explains it quite well Link as well as this video by Kevin Powell which demonstrates in practice when you would use which Link (NB. You can also combine them in an application, it all depends on the use-case)

    Also I've had a look at your code and semantically you could change this to just have the child elements as div and img inside the main tag, you don't necessarily need the article as this serves a different purpose Link

    Another thing - make sure to always locally host your fonts for privacy and performance reasons. Here's another good video which shows how to practically do this Link You can also download your typeface using Fontsource npm packages, then import the weights you require and reference them in your elements. Super easy and convenient Link

    Hope this helps!

    Marked as helpful

    0
  • @asbhogal

    Posted

    Hi Kartik,

    The reason why most of your pictures are showing is because of the / at the beginning of the src path. Remove these where they're present and your images will show.

    Just a few other things:

    • Locally host your Google Fonts for privacy and performance reasons. Here's a good link explaining why and how to do this Link
    • Use clearer variable and class names, ie. "notification" instead of "noti", as this can make it harder for other developers to read (always code with a mindset of others reading it to work on or debug it)
    • Make sure to add an alt attribute for screen readers and other assistive technologies (this helps them to understand the purpose and context of images for accessibility)

    Hope this helps!

    Marked as helpful

    0
  • Pascal 40

    @CodingEule

    Submitted

    Hello,

    not happy at all with the desktop version. Thankfull for any clue, how to improve.

    Thanks

    @asbhogal

    Posted

    Hi Pascal,

    This is a good attempt. I've outlined below what you need to do to get it closer to the mockups:

    • Firstly, remove the width: 100vw and width: 300px on your parent container. Neither should be used, as they cause overflow issues, which is what's happening here. You should set a max-width of, for e.g., 1000px and a width: 100% so anything below the max-width in the viewport will mean the content will occupy the full space it can.
    • You need to replace the current way the img elements are being used in the div with a <picture> element for them instead. Here's a link from MDN explaining this Link
    • Use CSS grid on the parent element for the columns and change this flow for viewports lower than 600px.
    • Locally host your Google Fonts for privacy and performance reasons. Here's a good video explaining this in further detail Link

    Hope this helps!

    Marked as helpful

    1
  • @jr19981010

    Submitted

    I'd love to see your constructive feedback upon my project

    Notification Page

    #accessibility

    1

    @asbhogal

    Posted

    Hi there,

    Good work! I can see you've made a solid attempt at matching to the mockups. I've noticed several things however which are worth considering for refactoring this:

    • You can use the font-clamp() property to dynamically scale down the font-size from a value set at a high viewport width down to the the lowest set viewport width in rem (note that this assumes a base font-size of 16px which is suitable generally, but it's worth noting in larger applications this would require further adjustments.) Here's a link to generate the function Link
    • Your markup is also semantically incorrect. The <header> should always be at the top, outside any other element (except the body) and your main content should be wrapped in the <main> element to mark where the most interactive content is (important for accessibility.) You could remove the two <div> elements and style the first two I mentioned.
    • It's great to see that you've locally hosted your font, however this should be in .woff2 format as this is designed for the web. Here's a link explaining this in further detail Link
    • Your <img> elements are missing alt attributes, which are important for screen readers and other assistive technologies to be able to read them to establish their purpose and context
    • Avoid using var when declaring variables - I noticed this in your var classname = span.getAttribute('class');. This should be const seeing as it isn't being reassigned within the same scope

    Hope this helps!

    Marked as helpful

    1
  • @rodrigompires

    Submitted

    Made with React, and some CSS animations with theme changes. Click on my avatar to open a modal.

    React

    #react

    1

    @asbhogal

    Posted

    Hi Rodrigo,

    This is a really interesting approach and it's great to see some out-of-the-box approaches, with rem units for font-size also used.

    I'd say in terms of typography, keep these consistent as the Elegant version has multiple versions across different cards. The font is also hard to make out on some of these cards across the themes because the background/foreground contrast ratio is very low, making it difficult in particular for screen readers and those with assistive technologies to read.

    Also, if you're using React I'd suggest deploying to a more robust platform like Vercel or Netlify which handles caching, performance and stability much better. They have CI/CD and debugging to identify errors during build and if you use Vite (more on this below) it will automatically build your application with each Git push, without needing to run an npm script.

    Following on from this, avoid using CRA as Meta have stopped supporting this. For client-side applications like this, use Vite, which supports vanilla, React, Angular etc.

    Locally host your Google Fonts for privacy and performance reasons. Since you're using an npm environment, you can install the Noto Sans typeface using Fontsource, import the weights you need and declare the properties in your elements. Here's a link to documentation Link

    With your directories, I'd suggest splitting your styles from your logic files by using separate folders (ie. a css folder with your component stylesheets, since you already have separation of concerns)

    Hope this helps!

    0
  • @webdevhill

    Submitted

    Hello frontendmentor.io coders,

    The provided background .svg for desktop is only 1440px. On xl screens, it obviously does not span the device's screen. There are resources for expanding the SVG view box but that seems beyond the parameters of the style guide. Any opinions on style guide parameters and expectations or solutions for extra large screens?

    Thank you

    @asbhogal

    Posted

    Hi Jeffrey,

    This is great work - well done!

    To handle the background images they should ideally be placed in a <picture> element in your markup, with paths to the mobile and desktop versions which change respective to a specified viewport width. I've added a link here from MDN detailing this and how to implement it Link

    Also, locally host your Google Fonts for privacy and performance reasons. Here's a good video explaining why and how to achieve this Link

    Hope this helps!

    0
  • @AslamtoIbrahim

    Submitted

    I don't know how to add suitable size to my elements adding width height min-height max-width I want to know more about px rem vw?

    @asbhogal

    Posted

    Hi Aslam,

    When you use sizing, you should use rem units. There are converters which use the base of 16px to convert these for you. In general with containers, you should use a max-width on the parent container with an explicit value (ie. unit) and a width: 100% with @media queries for padding with smaller devices. For eg., if your max-width is 350px, by the time you get down to that value in the viewport, the content will touch the sides, so you can add a padding property for the body element in a @media query for this width so you still have some spacing. The max-width means that with viewports above that width, it will always stay at that fixed value, and with viewports below it, the content will take up the full (ie. 100%) width of the parent container.

    You should also avoid setting explicit heights to the parent container and width and height properties to the child elements inside it because this can cause overflow issues. It's best to let them occupy the space they need to naturally.

    With this, hopefully you can refactor your code with these changes. If you need any further help, let me know.

    Marked as helpful

    1
  • @ibrahimherith

    Submitted

    Hi everyone,

    This is my solution to the calculator app challenge. I learned alot of things while doing it. Hope you enjoy it.

    Quick question though, how do you guys include one javascript file into another file? for example you have first.js and second.js files and you want to include both in main.js file, whats the trick?

    @asbhogal

    Posted

    Hi Ibrahim,

    Great work! In answer to your question, you export your functions and then import these as modules into another file (whether that's a second, or a main one.) This takes advantage of ES6, and conventionally you use a bundler to handle this, as vanilla environments can't handle module compilation without it. The easiest one to use in this case is Vite, which supports vanilla (pure HTML, CSS and JS) files, React, Angular etc. If you deploy these to Vercel, the platform will re-build your project with each Git commit. If you're new to these practices, I'd be happy to send you some links to documentation etc.

    Hope this helps!

    Marked as helpful

    0
  • @asbhogal

    Posted

    Hi Lorena,

    This is a very good solution, well done! The designs match the mockups nicely and it's great to see a React approach to get some early exposure to state management.

    Just a few pointers below worth considering when you refactor and for future builds:

    • For the if statement in your useEffect(), you can refactor this using the ternary operator, which is ideal for single-line parsing for truthy and falsy conditions: darkMode ? body.classList.add("dark") : body.classList.remove("dark"); here, if darkMode is true (by default by itself means it evaluates if it is truthful first), then parse the instruction after the ?, or if it is falsy, parse the instruction after :
    • Locally host your Google Fonts for privacy and performance reasons. Since you're using an npm environment, you can use Fontsource to install the typeface, then import the specific font-weights you require and then declare the typography via font-family, font-weight etc. in your stylesheets. Because you're using Tailwind, you can refer this in the theme or extend object in your tailwind.config.js file Here's a documentation link for the Inter font Link and how to customize the Tailwind theme Link
    • For client-side applications such as these, it's best to deploy to more robust platforms like Vercel or Netlify which handle caching and performance a lot better. They also include CI/CD and debug logging during build time to identify any issues, and Vercel automatically deploys a new build with each Git push, which means you don't need to run an npm script prior to committing.

    Hope this helps!

    Marked as helpful

    0
  • @rdonatiello1

    Submitted

    Hi Everyone -

    Thank you for checking out my solution and (hopefully) providing some valuable feedback. This is my first project so I really appreciate any helpful tips you can provide. The biggest challenge for me was centering the container that holds all of the elements. I ended up figuring it out but I feel like there's probably a much more efficient way to accomplish it. Let me know what you think.

    Cheers.

    @asbhogal

    Posted

    Hi there,

    This is a good attempt, and I can see why you've used certain code. I've made several suggestions below when you refactor this:

    • Avoid using the absolute to position this card and instead use flex properties (ie. display: flex; flex-direction: column) and margin: 0 auto to centralise it
    • You can style the body parent element of the card by also using flex properties and height: 100vh to vertically center the card (height: 100vh display: flex; flex-direction: column; justify-content: center;)
    • You should set a max-width: 320px, for e.g., on the parent container, and a width: 100%, which means any value above 320px will keep it fixed at that, and any width below will take up the full length (remember to convert these px to rem values)
    • Remove the max-width property on the child img element and replace this with width: 100%. You also won't need the margin property.
    • You can remove the .img-block element as this doesn't serve a purpose here (presumably this was to be the parent container which wraps the img? Either way you can remove this)
    • Use a CSS reset which removes all pre-set styling browsers add to web pages, which can cause cross-browser inconsistencies. Here's a good modern one which is recommended Link
    • It's best to use explicit (ie. rem) units for positioning elements, instead of % because these are more variable and prone to cross-browser responsiveness issues

    Hope this helps!

    Marked as helpful

    1
  • Bogdan Kim 780

    @semperprimum

    Submitted

    I would greatly appreciate your feedback!

    This was a tough one..

    • It was my first time building a carousel, and I used Swiper JS for it. The experience was quite challenging. I encountered numerous issues, such as flickering, and the carousel occasionally skipping some items. However, after investing several hours into it, I managed to fix the problems, and now it seems to be working properly.

    • The grid part, on the other hand, wasn't as difficult as i thought it would be. It was my first time incorporating grid-areas in the project, and I found them incredibly helpful. Using media queries with them was a breeze!

    Responsive Design Portfolio with a Carousel

    #accessibility#bem#react#styled-components#vite

    1

    @asbhogal

    Posted

    Hi Bogdan,

    This is a great solution - well done! You've used grid well here, as documented, as well as rem values and a reset, and its responsive. Nice. Just a few things I thought I'd mention which is more from a management/maintainability perspective:

    • With the Skills.jsx, to avoid repeated code, you can declare an array of objects with key-value pairs for the icon, text and classname (instead of using them as props) then map() over this array and render a <Card /> component for each, passing in the array index for the key prop and the respective three properties.
    • You've separated your variables, reset and utility classes, but I'd also recommend splitting your App.scss into individual components for easier management and debugging - it reduces cognitive load, as isolating a single element from a long stylesheet can take some time. You can search for it, but I t think its better to approach component styling the way you approach component logic and split them.
    • Locally host your Google Fonts for privacy and performance reasons. You can use the fontsource npm package to install the typefaces you need and then import the weights required, then declare the font-family where required. Here's their documentation. Link. I use this a lot and find it incredibly easy to work with. They install in the web-recommended woff2 format and you only end up with what you've used in your production build, and you can import them into config files for UI Component libraries too, making it very versatile.
    • In terms of SEO you're missing the meta description for crawlers to index your site page. While this is just a project, it's good practice to get into from the start so it becomes natural

    Also, from a personal perspective, I think using either styled components or SASS in a project is best, instead of combining them. While there isn't a hard rule for this and some would argue against it, having one approach (either coupled for single management or separated concerns) means it'll be easier to locate and debug styling and possible prevent code clashes. That's just my two cents.

    Hope this helps!

    Marked as helpful

    1
  • @asbhogal

    Posted

    Hi Jhonata,

    To make this responsive, you can either use flex-direction: column in a @media query or flex-wrap, or grid, which would retain the structure. The choice of when to use either depends on the content and how you want it to adapt, but here's a good link explaining the difference between the two. Link

    Before you do this however, I'd suggest making several changes to your code because these are causing issues to your layout:

    • Remove the margin-left on the section as this is causing issues with viewport width changes. Presumably this was added to try and centralise the container? In any case, use justify-content: center and remove the width: 50% on the .results .sumario.
    • You should also remove the width on the parent .conteudo container as explicit values shouldn't be set on this. Set a max-width instead, with a width: 100%, so anything below the max-width value will occupy the full width of the viewport and above, it will stay at the fixed max-width.
    • Change the height to 100vh to keep it vertically centered as, same as above, you shouldn't be using explicit values for these, as they constrain content and cause overflowing issues (ie. when content inside spills outside because the container isn't adapting as it has a fixed value set.)

    Hope this helps!

    0
  • @ofthewildfire

    Submitted

    Single Price Component Grid Solution

    Following the road-maps on Discord I have completed this challenge and my main focus was accessibility and the responsive aspect of the challenge. I am still learning accessibility and the topic is pretty dense, however, I did as much as I could on my knowledge, namely:

    • Layout not breaking or looking too weird when user changes font size on their browser/ and or zooms in. I tested this with Chrome, font-size at Very Large and zoomed in to 200% (as per W3.org links I read) and content shifted to maintain its form.

    • Font-size changing with view-port, I used clamp() for this, if there is a better way, please let me know.

    • I changed the colours, they were failing colour contrast checks, so I did my best to pick colours that still looked decent and matched. They now pass those checks.

    Things I Need to Fix

    • There is an alignment issue with the "Why Us" box, the heading is at a different spot compared to the other heading in the "Monthly Subscription" box, and it makes it look off, I will keep working on a solution, I suspect its to do with the way I am aligning items in the boxes with flex-box, will continue to try and fix it. Advice appreciated of course though. <3

    Unsure about

    • I am very unsure about keyboard accessibility, I went heavy focus on trying to make the fonts size and the containers change, but, the keyboard part, I didn't fully understand it all, and so, I dont think that is implemented, advice there would be appreciated. ^_^ as well as other things in this area too!

    Since I am still learning and things may be totally whacky any and all advice is greatly appreciated - thanks, and have a good day.

    @asbhogal

    Posted

    Hey Kaycee - this is very good, well done!

    I really like the changes you've made to the palette and considerations to accessibility which often get overlooked at times unfortunately in front-end dev. You've documented your process nicely and your code is well organised. Props to you.

    In terms of keyboard accessibility, the main thing is to ensure the button is focusable when visitors use the tab on the website to navigate through. In order to do this, it's best to remove the transparent value on the outline as this removes the focus on the element so they don't know if it's been selected. Here's a good link which might help illustrate this Link

    With regards to the heading alignment issue, I suspect this is due to the space-around value assigned to both containers, as this calculates gaps both in between the child components and outside. It does this based on the height of the container against the total height of the elements and equally distributes the difference. The spacing that the flex has created on the outside of the right is greater than the left, thereby causing the headings and list section to be 'pushed in' and subsequently lower than the heading on the left, if that makes sense. I've added a link here explaining this in further detail Link In this case, you could try removing the justify-content altogether (which will default it to the space-between from the main>.grid_item declaration, pushing them to the edges of the top and bottom) and adding a specific padding to these two, e.g. padding: 1em 1.5em which will set an equal amount and thereby keep the headings the same height from the top.

    You could also change the background-color to #205857 to flow nicely with the right container and change the colors of the h1 to #205857 and the h2 to #197371. Visually I think this blends better and keeps the color palette to a simple accent of two tones and white.

    I've ran your site through an online accessibility checker for WCAG 2.0 (Level AA) and Level AAA and found no potential or known issues - great news. Here's the link to the checker which you can also use Link

    Hope this helps, and again, really great job. Impressive work!

    Marked as helpful

    0
  • @asbhogal

    Posted

    Hi Isidora,

    Great work! The accordion seems to function nicely. I've noted some things down below worth considering for refactoring:

    • The accordion isn't fully responsive - between 481px and 640px you have some overflow issues and spacing caused by the explicit height value set on the parent container. Ideally you should avoid this, as it constrains the child elements. It's best to let them occupy the space they need naturally.
    • I'd recommend using grid for this to keep the intrinsic values of the left and right columns consistent as the viewport changes. This ensures the content isn't compressed. Here's a good link explaining the difference between the grid and flex and when to use one instead of the other Link
    • Use a CSS reset to ensure it removes any preset styling by browsers and defaults to a completely unstyled 'canvas'. Here's a link to a good modern one Link
    • You can refactor your JS logic by replacing the if statements with a ternary operator, for e.g. addSupp.addEventListener("click", () => { paraFive.style.display = (paraFive.style.display === "none") ? "block" : "none"; hideParaTwo(); hideParaThree(); hideParaOne(); hideParaFour(); }) This says that if paraFive.style.display is equal to "none", it sets paraFive.style.display to "block", otherwise, it sets it to "none", and is great for single-line parsing for when the condition is evaluated (ie. if it is truthy, it returns the "block" for the display and if its falsy, it returns "none")
    • Your code also has a lot of gaps and spaces, particularly the stylesheet. I'd recommend using an extension like Prettier if you use VS Code, which formats your code nicely and removes extraneous spacing. Here's the link to it Link

    Hope this helps!

    Marked as helpful

    1