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
Log in with GitHub
OverviewSolutions26Comments58

All comments

  • Yazdun• 1,310

    @Yazdun

    Posted

    This is looking great, well done!

    I have a suggestion for you, instead of hardcoding accordion data, you can store all the accordion data inside an array and map through the array inside your react component.

    This way you won't need to repeat the same code with different title and description, it also will be so much easier to update the data or change the JSX since the pattern is repeated only once.

    I've already opened a PR which resolves this issue, make sure to check it out.

    Other than that, it's looking fantastic, keep up the incredible work!

    Marked as helpful

    1
  • Shonuff• 280

    @TheShonuff

    Submitted

    Used this challenge as an opportunity to learn Svelte. Very happy with the framework and will continue to use it in the future. If you have any thoughts or critiques I would be happy to hear them. I always enjoy learning from my mistakes. Thank you for looking at my project.

    Planet Facts

    #svelte

    2

    Yazdun• 1,310

    @Yazdun

    Posted

    Hi Joe, great job on the challenge. This looks very clean and it's also responsive on all sort of devices. I specially love the animated hamburger icon 😁

    I don't know Svelte so I can't talk about your code, but there is a really serious issue in this project which you should be aware of.

    Imagine this is a production level project, and our user is doing a research on the Jupiter. She wants to bookmark Jupiters internal structure so she can come back to it later and use it as reference, right now there is no way to point exactly to Jupiters internal structure and she must first land on the website, then click on the Jupiter, then click on the internal structure ! We will definitely lose our user right there !

    Now imagine another scenario, Someone is reading Uranus surface geology and he finds it very interesting so he decides to share it with his friend, Now whatever URL he sends to his friend, they will land on Mercury overview 😁

    This can also completely destroy the SEO since there is no way for search engines to show exact results to the users.

    The solution to this issue is, creating separate pages for each planet and then creating sub pages for each category, for example : planets.com/earth/structure . Or if you don't want create pages, you must at least implement queries so users and search engines can point to a exact planet and sub category, for example : planets.com/solar?planet=earth&category=geology .

    Overall, Great job and keep coding 👏

    Marked as helpful

    2
  • Vanza Setia• 27,855

    @vanzasetia

    Submitted

    Hello Everyone! 👋

    I finally finish my first API challenge! 🥳

    This is a hard challenge for me, but I learned a lot of new things. 😀

    Hopefully, I can get some feedback as well since I am still new at asynchronous programming. 😁

    Now for the questions:

    • There's an article where it tells me that I should use input with type="text" instead of type="search". But, is this a good thing to follow?
    • Should I put the role="search" on the form element or on the div element (inside the form element)? I don't know the correct answer because MDN recommends putting the role="search" on the form element. But, Ahmad Shadeed recommends to put the role="search" on the form element to prevent overriding the default form semantic.
    • I follow the GitHub alternative text of the user's avatar which is like alt="@username". But, do you think it's good enough or is there a better alternative text for the avatar?
    • If you see the site, there are some statistics such as the total of public repositories, how many followers the user has, etc. My question is, should it be read as "8 Repos" or "Repos 8"? Currently, screenreaders will read the text as "Repos 8". But, if I should change it, then I would probably use flexbox to reverse the text visually. So, screenreaders would read the text as "8 Repos".
    • Should I have a hidden heading above each list? For example, <h3>User statistics</h3>. I got the inspiration from the @grace-snow's solution for the "Profile card component" challenge).
    • There are disabled links (or not available links) in this challenge. So, let's say the janedoe's GitHub account has no link in his/her profile. So, what do you think about the way I handle the disabled links? Currently, I make them as normal text (<span>Not Available</span>).
    • Also, you might notice that janedoe has no name. So, I visually hide the h2 if the user has no name, <h2 class="sr-only">Not Available</h2>. What are your thoughts?
    • Is there a better way to manage the colors for the dark mode and light mode? I used CSS variables in this project to make it easier because I could just switch the colors with prefers-color-scheme. You can see the CSS code to see how I handle the color for this project.
    • If you can help me suggest a better name for any function in my JavaScript, I would really appreciate it. For example, setThemeSwitcherStateForDarkMode() is a very long name. 😆

    I am aware of the HTML issues that have been generated. I added the role="list" to each ul element to make sure the ul element still has the semantic meaning when I set list-style: none;.

    If you find any bugs or notice I miss something, please let me know. Also, if you spot any bad practices in my code, feel free to tell me. I will try my best to make improvements to make this solution better. 🙂

    Also, if you have finished this challenge and would like me to give feedback on it, please include a link to your solution. I would be glad to help you! 😀

    Thanks!

    GitHub User Search App | HTML CSS Sass JavaScript (Async Await)

    #accessibility#bem#lighthouse#sass/scss

    7

    Yazdun• 1,310

    @Yazdun

    Posted

    Hi Vanza, Great job on the challenge, I specially love the fact that you added a query in the url for the searched user, that's a really nice touch.

    Here is quick issue that Matt Seidel noticed on my solution and then I could fix it. Try searching mseidel819 in your app and you will see a weird overflow which doesn't look nice, you may wanna take care of that.

    3
  • Andro87• 1,370

    @Andro87

    Submitted

    This is my solution for this multipages website. Any feedback on how to improve it will be highly appreciated. ;)

    Planets fact site

    #next#sass/scss#typescript#react

    1

    Yazdun• 1,310

    @Yazdun

    Posted

    Hi Andro, Great job on this challenge, This is actually one of my most favorite challenges on the frontend mentor. There are three main issues with this solution :

    • Your page's layout will break on wide screens.
    • Your project is not structured properly.
    • You took a wrong approach ( I explain why in depth )

    Now I'm going explain each issue.

    1. Page's Layout : I started with this one since it's the most easiest to fix. You have written a scss class called main_container inside ./styles/Home.module.scss and used it in various places.

    .main_container {
    width: 100%;
}

    Now, You've applied this class to the elements which are block level by default and taking 100% width is their default behavior, There is no need to apply this class to them.

    But what happens if the user is browsing our website in an ultrawide monitor ? Do we want our container to fill the whole screen ? The answer depends on the project itself, but for this project the answer is clearly no, because we don't have much content on the screen so it's better to control the container's width, so you can easily update main_container :

    .main_container {
    width: 100%;
    max-width: 80rem;
    margin: auto;
}

    With the above's approach, our container fill 100% of the available space but when it hits width of 80rem, it will stop growing and we will have a better looking layout on a widescreen.

    Also you need to use background svg which was provided with the starter files.

    2. Project's structure : This mainly comes back to components directory but I'd like to mention some more issues that I would've personally change :

    • You've used SCSS for styling but you haven't used any powers of SCSS, so why not use plain CSS instead ? If you are using SCSS it's better to at least embrace some of it's powers, if not, then it's easier to remove the SCSS complexitiy from our project and use simple plain CSS instead and sharpen our fundamentals. CSS itself is super powerful !

    • It's better to create a specific folder for each component so our codebase becomes more readable to other developers. You approach is working and it's fine, But in the eye of another developer who wants to contribute to your codebase, it's extremely cluttered. So it's better to put each component inside it's own specific folder.

    • It's better to create an index.ts file inside ./components and export all components from there, so in case we want to use multiple components somewhere else, we can import them all at once from ./components/index.ts.

    • Since we are using Nextjs, Vercel highly recommends us to make use of Absolute Imports and Module path aliases. It's really easy to configure and all you need to do is adding "baseUrl": "." to the compilerOptions inside tsconfig.json.

    3. You chose a bad approach

    This is the most important one and this can really hurt your project, Now you've used a fancy carousel and it has awesome transitions, but you've created a huge issue with choosing fancy stuff over performance ! Here is why :

    • What if our users wants to share surface geology section with their freinds ? No matter what url they send to their friends they will land on planet's overview when they click on it.

    • What if someone search for venus internal structure on the internet and they don't care about venus's overview or surface geology? Shall we expect them to land on overview and then look for internal structure button and manually click them?

    • What if our user is making a research on planet Mars and she wants to bookmark Mars's surface geology so she can return to it later for multiple times? Each time she must land on overview and then choose her desired tab which becomes annoying overtime and we will definitly lose our user right there!

    The solution to this is, creating a sub route for each specific section, There will be a main page for the planet which shows the overview, and other tabs should be a separate sub page on it's own, so search engines and users can make better use of our website.

    If you want to stick with the carousel approach, You must at least implement a functionality so we can point to the specific carousel tab with the page url ( You can use a query for this ).

    Overall you did a great job, I hope this helps you to improve your coding skill. I also opened a PR to your repo which fixes some of these issues that I mentioned.

    Marked as helpful

    3
  • P
    Dilhan Boca• 180

    @dboca93

    Submitted

    Hello,

    Feedback welcome for my first frontend mentor task. I'm new to web-dev and I'm keen to learn more. I imagine there are easier ways to complete this task using CSS Grid (which I'm currently learning) -- is this correct?

    Also, could someone comment on whether the sylistic changes I made on smaller screen sizes (via media queries) are correct ? Is the read-me file ok ? All other feedback is more than welcome. Thanks !

    QR Code Component: centered vertically and horizontally using position

    2

    Yazdun• 1,310

    @Yazdun

    Posted

    Hey man good job on the challenge. This project doesn't need any grid so it's fine the way you've done it ( although I can argue that it doesn't need any flexbox either 😅 ).

    The other thing is I don't see any need for media queries in this challenge. instead of using static width for the main__container , you can use something like :

    .main__container {
  width: 100%;
  max-width: 270px;
}

    With the above's approach, You won't need any media queries, main__container will occupy 100% of the screen and when it reaches 270px width, It will stop growing. You can give some paddings to the parent element to prevent main__container from hitting the screen's edges on mobile devices.

    Overall you did great, Keep it up.

    Marked as helpful

    0
  • Shonuff• 280

    @TheShonuff

    Submitted

    This is was an interesting challenge and had a lot of little things I didn't notice when I first accepted this challenge. This was a fantastic learning experience and looking forward to another intermediate level challenge.

    Tic Tac Toe

    #react

    1

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Joe, Congrats on finishing this challenge ! While the project itself looks awesome, there are some issues in regard to your react's codes which I like to mention :

    • First of all, remove the files which you are not using in your project from the source code, for example you have not written any tests for your project, so you can remove the App.test.js and setupTests.js so people won't get confused when they are exploring your repo.

    • You must create separate directories for different purposes, Now all the resources are living in src directory, This is working, there is no issue with that, But in the eye of the another developer who wants to use your project this looks extremly cluttered. My recommendation is creating /components directory and put each component inside in it's own separated folder, for example /components/game/Game.js. No matter how small our project is, we must follow the best practices anyway.

    • For exporting and importing files, The best practice is creating an index.js file inside each directory and export them once from it's directory, and then importing them all at once from there. To make this more clear I give you an example : You have imported assets in each file by pointing at them like

    import Xicon from "./assets/icon-x.svg";
import Oicon from "./assets/icon-o.svg";
import logo from "./assets/logo.svg";

    Now, what if at the middle of the project we decide to change the icon-x.svg name to icon-y.svg ? You guessed it, all hell will break loose ! We must go and find each folder which uses Xicon and manually change it's name to Yicon and also fix the import path. The better way is isolate the import inside it's directory so each time we want to change something's name, we only change it at one place. So here is my recommended approach :

    // ./assets/index.js
export { default as Xicon } from "./icon-x.svg";
export { default as Oicon } from "./icon-o.svg";
export { default as logo } from "./logo.svg";
export { default as IconRestart } from "./icon-restart.svg";
export { default as OiconOutline } from "./icon-o-outline.svg";
export { default as XiconOutline } from "./icon-x-outline.svg";

    Then inside the component :

    // ./components/someComponent/SomeComponent.js
import { OiconOutline, Oicon, Xicon, XiconOutline } from "../../assets";

    • Now let's talk about structuring your JSX. First of all you have to keep something in mind when working with react, I kinda made it a rule for myself : If your component is exceeding 100 lines of code, something is wrong ! . No one tells you that, React won't throw any error, and at that moment everything works fine, but when you come back to it after a week or so, or when you need to add a feature to your app after some time, It becomes a nightmare ! ( trust me on that I learnt it the hard way ). Each component should ONLY DO ONE THING at a time, For example I'm looking at your Board component, This is the sacred temple of your project and no one has the guts to touch this file 😅 it's 253 lines of code and there are all sort of functions, loops, 11 useStates, many if and else statements and ternary operators, and then you have passed all these stuff as props to different components so if we touch anything the whole app breaks. Here is my solution for this issue, You need to isolate the game's logic inside a context and then provide it to the components which needs them. Each component should interact with the context on it's own term and should not depend on other components in order to work, this way you can split Board.js into many small sub components so you can easily make changes to them and maintain your app.

    • For styling, I recommend you to learn CSS modules alongside with classnames npm package, Using global css may cause conflicts with css classnames and cause bugs in your app, with css modules you can make sure that css classnames are isolated inside component's scope.

    • I saw you have used react-device-detect library, This library has very special usecases and for your project you could've easily created a custom useWindowWidth hook ( You can learn more about this on google, it's really easy to create ) and show components based on user's device width instead of using a library.

    ✅ That's all I had to say, I also opened a PR to your github repo which fixes some of these issues. I hope this helps you to improve your react coding skills. Overall great job and keep it up

    Marked as helpful

    2
  • P
    Fluffy Kas• 7,735

    @FluffyKas

    Submitted

    Hey guys,

    I'd like to apologise to anyone who's only interested in the rating card component. Just click through the pizza creation, on the finish page you can see it as a popup, if you choose to give a feedback.

    Because that's what this component turned into. A pizza app. Somehow. I wanted to practice React so I kept adding new things to it and this is what I ended up with. Design wise it's not the fanciest (I'm really not a designer, sorry) but that wasn't really the point here anyway.

    I added a couple of features: a pizza creation function, signup/login function with Firebase (password reset as well), a light-dark theme, a contact form and a pizza-loader animation (thanks to my sister for the pizza images she made for this and for the home page background). And finally there's the rating card component as a modal, which was the original challenge.

    I learned a lot in the process: I got a tiny bit better at Framer Motion, I wrote a bunch of React code, used context and Firebase for the first time. I even managed to reuse some things from my other projects (like the mobile menu) - it felt great that I didn't have to do it again from scratch.

    I appreciate if you take the time to look at it. I take any sort of feedback regarding accessibility, React best practices, css (a bit messy at the moment), animations, how to make things more reusable... anything.

    If there's anyone who's a pro at Framer Motion and can give me a hint on how to do exit animations, I'd love it.

    And lastly I hope it isn't inappropriate to upload this app here. It really started out as the rating card component, just got carried away.

    Anyway, happy Easer/have a good weekend everyone!^^

    Interactive Rating Card - turned into a modal in a pizza app

    #firebase#framer-motion#react#react-router#sass/scss

    5

    Yazdun• 1,310

    @Yazdun

    Posted

    Great job on this challenge, everything is smooth and awesome 👏

    In regards to your code, There are some issues that I like to mention.

    • Components which are used as react router element, should be separated from other components. This is not crucial but it helps us to maintain our codebase easier. Usually we create a folder called /pages or /views and put our page components there.

    • For importing and exporting components, it's better to first reexport them from an index.js file inside components folder, and then when we want to import them somewhere else, we can import them all at once. To make it more clear look at the following example

    ⛔ BAD PRACTICE

    //APP.JS

import Header from "./components/header/Header";
import Home from "./components/home/Home";
import Base from "./components/base/Base";

    ✅ GOOD PRACTICE

    // components/index.js

import Header from "./header/Header";
import Home from "./home/Home";
import Base from "./base/Base";

export { Header, Home, Base };

    //APP.JS

import { Header, Home, Base } from "./components";

    This way you'll have much more cleaner syntax and also it is easier to maintain 👆

    • When multiple components are using same values, we MUST use context to avoid prop drilling. For example themes are something that are used globally in our app so we shouldn't pass it as a prop, we should isolate it inside a context and each time a components needs it, we can provide theme to it, Here is a theme context that I wrote for your app, by using useTheme() you can use theme and setTheme value across your whole application :
    import { useContext, createContext } from "react";
import useLocalStorage from "use-local-storage";

const ThemeContext = createContext();

export function ThemeProvider({ children }) {
  const defaultDark = window.matchMedia("(perfers-color-scheme: dark)").matches;
  const [theme, setTheme] = useLocalStorage(
    "theme",
    defaultDark ? "dark" : "light"
  );

  return (
    <ThemeContext.Provider value={{ theme, setTheme }}>
      {children}
    </ThemeContext.Provider>
  );
}

export function useTheme() {
  return useContext(ThemeContext);
}
    • We have same prop drilling issue for your pizzas too ! Right now you're drilling newPizza and setNewPizza into four different components, and this is red flag ! Here is a context which handles our pizzas so we can have access to newPizza and setNewPizza across our whole application :
    import { useState, useContext, createContext } from "react";

const PizzaContext = createContext();

export function PizzaProvider({ children }) {
  const [newPizza, setNewPizza] = useState({ base: "", toppings: [] });

  return (
    <PizzaContext.Provider value={{ newPizza, setNewPizza }}>
      {children}
    </PizzaContext.Provider>
  );
}

export function usePizza() {
  return useContext(PizzaContext);
}
    • As a developer, we must follow DRY principle to keep our code maintainable, repetition is a red flag whenever and where ever we see duplicated code, we must figure out a way to fix it. For example inside your RatingCard component, You've repeated the same input for 5 times with different values, this is a bad practice. Instead you can create an array of inputs ( ideally react components ) and loop through them using map() method, Here is my refactored version of your RatingCard :
    const nums = [1, 2, 3, 4, 5];

<div className="input-container">
  {nums.map((num, index) => {
    return (
      <div className="focus-box" key={index}>
        <input
          type="radio"
          name="rating"
          id={num}
          value={num}
          onClick={selectRating}
        />
        <label htmlFor={num}>{num}</label>
      </div>
    );
  })}
</div>;

    • It's better to keep react components under 100 lines, I'd say 60 or 70 lines is ideal, but as soon as your component exceed 100 lines, it's time to break it up to separate components. Although It's fine to sometimes have long jsx.

    • Right now there is way too much repetition inside your ContactPage, Each input should be separate component. I recommend you to learn React Hook Form, This library has made my life 10 times easier.

    • Use SCSS modules instead of global SCSS

    • You can use React icons for your icons, it's easier and more reliable than using external assets IMO.

    ✅ Also I opened a pull request to your repo which fixes most of the issues I mentioned, Also you can find this comment there, inside ISSUES.md.I Hope this helps.

    Marked as helpful

    10
  • Vanza Setia• 27,855

    @vanzasetia

    Submitted

    Hello Everyone! 👋

    Finally, I completed another PREMIUM challenge! 🎉

    It was a challenge where I needed to play around with the background images. They were tricky! I also improved my old Regular Expression from the Base Apparel challenge. I had written everything that I learned in the README file. So feel free to check it out! (and give feedback on it 😄)

    Now for the questions:

    • I added alternative text for the Spotify, Apple Podcast, Google Podcasts, and Pocket Casts logos. However, the paragraph above the form element has mentioned all the platforms. I'm not sure that they are decorative or informative images. So, what do you think about it?
    • If you try using a screen reader on my website, can you understand the page content?
    • Can you navigate this website comfortably using your keyboard?

    Any questions on the technique that I'm using are welcome! 😁

    Also, if you have finished this challenge and would like me to give feedback on it, please include a link to your solution. I would be glad to help you! 😀

    Thanks!

    Pod Request Access Landing Page | HTML CSS Sass JS (Regex)

    #accessibility#bem#sass/scss#lighthouse

    2

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Vanza, this is so awesome man well done, your commitment is so inspiring keep it up 👏 I played around with your solution for a little bit, and as perfect as it is, I have some suggestions :

    • for error handling, I personally prefer to handle errors before submission, I declare a variable called let hasSubmitted = false; and as long as it is false, it means user has not submit the form yet, then after first submission, if there is any error, I show the error and also hasSubmitted = true; . this let me to show errors dynamically after failed submission as users type their email by using something along the following code 👇
      heroInput.addEventListener("input", e => {
    hasSubmitted && // handle errors here 
  });

    and also I disable the button as long as there is any error after first failed submission, so users won't need to click submit to find out if there is any error or not and errors will be handled as they are typing. I know this is a personal preference but I think it had worth mentioning.

    • also there is another thing : using nested if and else statement, again that is not really an issue but I'm gonna tell you my personal approach and maybe you find it useful. in your js you've used 👇
     if (isEmailInvalid) {
      event.preventDefault();
      if (heroInput.value) {
        showAlertMessage(message);
        setTimeout(hideAlertMessage, 5000);
      } else {
        showAlertMessage(message);
        setTimeout(hideAlertMessage, 5000);
      }
    }

    but I think using switch instead of nesting if and else is more readable, maybe you find following code more readable too 👇

      switch (isEmailInvalid) {
      case false:
        console.log("it's all good");
        break;
      case heroInput.value:
        showAlertMessage(errorMessage);
        setTimeout(hideAlertMessage, 5000);
        break;

      default:
        showAlertMessage(errorMessage);
        setTimeout(hideAlertMessage, 5000);
        break;
    }

    that's all I had to say and I love to know what you think about em. and here I have a question :

    • how much time do you spend on creating these challenges ? your README file is so nice and I haven't seen such a clean solution on frontend mentor, and do you still do these challenges on a mobile device ? forgive me for my curiosity though

    Marked as helpful

    2
  • P
    Kristina225• 260

    @Kristina225

    Submitted

    Hi, everyone. Any feedback you can give me on my solution is appreciated. I was mostly struggling with the hover state on the image. I added an empty <div> in my HTML for the cyan transparent background and changed its 'display' property with 'mouseenter' and 'mouseleave' with JavaScript. But maybe there are better ways and JavaScript is not necessary. Thank you all.

    NFT card component using HTML, CSS and JavaScript

    4

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Kristina, well done on this challenge 👏 I see you've written extremely clean code, thumbs up 👍

    • about solution itself, as you said yourself, you don't need JS for hover stats, all you need is :
    .card__image--container:hover .card__image--backdrop,
.card__image--container:hover .card__image--icon {
  display: block;
}

    in your css and you'll achieve same functionality without any javascript.

    • here there is another issue I found, you've used width: 35rem; and height: 59.6rem; on your card, so when device's width gets smaller your layout will break, it is not recommended to use static size for your elements and you must let the content specify the size, so what you can do is :
    .card {
  width: 100%;
  max-width: 35rem;
  // rest of the code ...
}

.card__image {
  display: block;
  position: relative;
  /* let the content specify width */
  max-width: 100%;
  border-radius: 0.8rem;
}

    and some padding on the body, will make sure that your layout won't break on small screens.

    • use PRETTIER extension on your code editor to format your code.

    • side issue about github 😄 : don't jam all your solutions on one branch, create separate branch for each solution, and as you are already using netlify for deployment, you can easily deploy each branch automatically using netlify, this way if you mess up one of your project, your whole repo won't go bananas and you can easily delete the corrupted branch.

    I also opened a pull request to your repo which will fix the solution issues I mentioned, keep coding

    Marked as helpful

    2
  • Tomas Scerbak• 720

    @TomasScerbak

    Submitted

    I have no idea how to combine HTML, CSS and JS to show different layouts when click on share button :D. Could only finish mobile version.....somehow.

    Any suggestion for best practice and how to do it would be appreciated.

    Thank you

    article-preview-component-master

    1

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello dear Potato 🥔

    allright there is so many ways to get this done, I mention two main approaches I personally take for these scenarios.

    • for this challenge, I used media queries. I mean it's a same component, as long as it is within small screens it sticks to the bottom, but as soon as it hits wider screens, it completely transforms into different styles, but it is still same css class ! here is my solution on this challenge in case you wanna check it out.

    • there is another approach you can take, you can create two different elements for mobile version and desktop version, and add display:none to desktop version, then using media queries, when user reaches wider screen you can add display:block to the desktop version and display:none to the mobile version. ( I guess it got a little bit confusing but I did my best 😁 ). then using javascript you can toggle both these classes but always one of them is hidden according to the device's screen width.

    I hope you find this comment helpful

    Marked as helpful

    0
  • Yazdun• 1,310

    @Yazdun

    Posted

    Hey Raul great job on this, to get rid of accessibility issue, you must add a h1 heading to your main tag which tells what this page is about, it can be hidden, you can use a simple sr-only css class if you want to make it hidden.

    right now, as your website hits around 337px width, QR card sticks to the edge of the screen which is not the best user experience IMO, so make sure to use some paddings on QR code's parent element.

    in my opinion your box shadow is too thick, but hey, that's a personal opinion 😁

    well done and keep coding !

    Marked as helpful

    0
  • lmac-1• 10

    @lmac-1

    Submitted

    This is my first challenge on the platform. I chose it because it didn't look like I needed to do too much to make it responsive, and wanted to start with something simple. Overall I really enjoyed it, it was a big challenge for me but I'm happy with the final results.

    Looking for feedback on the following areas:

    1. Use of semantic HTML - I'm not confident I've chosen the correct elements for the design
    2. General CSS good practices - is my CSS well written? Am I missing things?
    3. Background position - I was quite stuck on this property and I'm not completely confident with this part of my solution. Is there a better approach?

    Profile card component built using HTML and CSS

    2

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Lucy and well done on the challenge, Here are my suggestions :

    • Use Prettier extension on your IDE to format your code.

    • use code below instead of specifying default value for html element separately

    *,
*::before,
*::after {
  margin: 0;
  padding: 0;
  box-sizing: border-box;
}
    • Html elements are block level by default ( thats one of the reason for approaching mobile first ), so you don't need 👇
    display: flex;
    justify-content: center;
    align-items: center;
    flex-direction: column;

    on your main tag, you get the column by default

    • give min-height:100vh instead of height:100% to your body.

    • right now, background circles are not responsive and layout breaks quite easily on smaller screens, so here is what I did on this challenge for background circle to make sure of their responsiveness. firstly add 👇

    .c1 {
  right: 50%;
  bottom: 43%;
}

.c2 {
  top: 50%;
  left: 50%;
}

.c1,
.c2 {
  position: fixed;
  transition: 0.5s cubic-bezier(0.075, 0.82, 0.165, 0.6);
  z-index: -1;
}

    to your css and then add images to the markup 👇

      <img
        src="./assets/images/bg-pattern-top.svg"
        alt=""
        role="presentation"
        class="c1"
      />
      <img
        src="./assets/images/bg-pattern-bottom.svg"
        role="presentation"
        alt=""
        class="c2"
      />

    and then remove background properties from body. this way, background circles won't go bananas on smaller screens. keep in mind that's my approach and there are definitely better approaches you may come up with.

    • use width:100% and then max-width: ... px instead of giving a static width to your card, so card's layout won't break on small devices, right now your card leaves side scroll on small screens.

    • I don't recommend using static heights for elements unless it's really crucial.

    • for stats, I think you'll be better off with something like

     <ul class="card-stats">
        <li><span>80k</span> <span>Followers</span></li>
        <li><span>803k</span> <span>Likes</span></li>
        <li><span>1.4k</span> <span>Photos</span></li>
 </ul>

    instead of using so many divs.

    • give some paddings to body so content won't occupy whole screen on mobile devices.

    ✅ Also I opened a pull request to your repository which will fix these issues

    I hope this was helpful

    Marked as helpful

    2
  • anna• 340

    @annab6

    Submitted

    This challenge was quite difficult for me. I haven´t managed to create a small "triangle" under the sharing menu in the desktop layout. Any ideas on how to add it? Any feedback on what I could improve is very welcome!

    Responsive component with mobile-first approach

    2

    Yazdun• 1,310

    @Yazdun

    Posted

    Hi Anna and well done on the challenge, for issue you mentioned, you must use before css pseudo selector to add the triangle, something like :

     .card__share-desktop::before {
    content: "";
    display: block;
    width: 20px;
    height: 20px;
    background-color: var(--dark-grey-blue);
    position: absolute;
    bottom: -10px;
    right: 100px;
    transform: rotate(45deg);
  }

    also use Prettier extension on your IDE to format your code.

    ✅ I opened a pull request to your repository which will fix the issue.

    I hope this was helpful

    Marked as helpful

    1
  • Bernard• 195

    @Bernardanxiety

    Submitted

    Should i even be trying to make these challenges responsive or just work hardcoded on 375 and 1440 sizes? I don't like the fact that i add all-around padding to make it look good on desktop but then it's impossible to make it responsive on smaller sizes unless im just really bad at css and close-minded. I also feel like i have some problems with flexbox and it was hard for me to make the image and the left panel hit their desired sizes. Looking for every bit of advice so i can improve and continue my journey with frontend, thanks in advance!

    Coming soon page mobile first using html css js

    1

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Bernard ! Of course you should try to make challenges responsive 😄 So you can learn it and apply it to real life projects, It may seem disappointing at beginning but as you progress, it becomes much easier ! don't worry about it and just keep trying.

    Let's talk about the solution:

    • as you know It's not responsive right now it breaks very easily, but layout looks good on smaller devices, I took a quick look on your code and I think you don't need to use grid and you'll be fine with flexbox, Here is very good resource so master flexbox in no time !

    • Use Prettier extension on your IDE to format your code.

    Apart from that, Your code looks clean and nice and well done on that.

    ✅ Also I opened a pull request to your repository which will fix responsiveness a little bit, I hope this helps and keep coding !

    0
  • Ctrl+FJ• 750

    @FlorianJourde

    Submitted

    Big s/o to @Yazdun, who gave me some tips about CSS animations. By the way, I guess I should go through SCSS, since my stylesheets begin to be veeery long ! I'm particulary happy about my .wrapper system, which avoid me to use Bootstrap. I think I'll reuse this .wrapper in the future. Feel free to take it, if you judge it relevant ! Also, this little tool helped me to crop .svg image.

    Place to the questions, now :

    • Can I inherit from upper @media query ? Or is it more secure to inherit from the original one ?
    • If I can't, what would be the best practice to position backgrounds without to use media queries, taking resizing in consideration ? Background-position ?I'm mostyl talking about white sections.

    Hope you'll like it ! I had some fun playing with those kind of "breathing" effect ! I didn't spent much time in the .js, but it seems functional.

    Blogr landing page solution with HTML, CSS, JS

    2

    Yazdun• 1,310

    @Yazdun

    Posted

    This motivates me to pick up this challenge 😃 layout is responsive and functional and looks very solid, If you need help on accessibility I'll be glad to help, You have some issues that I haven't encountered yet so maybe I can learn something myself, Your call anyway.

    • About media query question, only talking about inheriting from upper media query makes me confused, I have never seen it before but I'm not sure if it's possible, but even if it is, It doesn't sound like a good approach IMO.

    • About background issue I certainly need some help myself 😅

    • If you want to start using sass, here is a great folder architecture which saved me from a lot of headaches, there are so many folder structures out there but this one works for me the best. They also provide boilerplate which you can find here. If you were interested in this, I use same structure on my repo which has all the media query mixins based on freecodecamp's breakpoint guide, also I added sr-only,hide-for-mobile,hide-for-desktop classes and removed some default styles which saves me from lot of repetition in the long run, feel free to use it on your own projects. (here is the repo)

    I hope this helps and keep coding 👏

    Marked as helpful

    1
  • Logan Willaumez• 160

    @LoganWillaumez

    Submitted

    Hello ! Next challenge I did here ! 😁

    It's begin to be more and more clear on my head how to approach a project and deal with it.

    I don't understand very well the accessibility issue I have.. Maybe someone can explain me?

    If u have any feedback, don't hesitate 😃

    Best,

    Logan

    Ping Single column-coming soon page challenge hub

    1

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Logan this looks so cool 😁 I love the vibrate thing on error input, It would be cooler if you add a success alert too, regarding to your question :

    • Each page should have at least one main tag which wraps the whole page's content and this main tag, must have a level one heading which tells what this page is about, this h1 can be hidden. Fix this and your accessibility issues will be gone. Don't forget to generate new report though !

    Overall well done and keep up the good work 👏

    Marked as helpful

    1
  • Yazdun• 1,310

    @Yazdun

    Posted

    Hello Christopher ! The icon issue is probably <img src="/images/..."> change them to <img src="./images/..."> and see if it works on github pages

    0
  • Ildrim• 50

    @Ildrim

    Submitted

    Hello this is my second project, but i really struggled on this one, and i seriously need some feedback, since i feel that my code is really messy on this one.

    My main problem was that the transition from mobile to desktop (since im working mobile first) using flexbox was a bit weird as far as responsiveness go. The image would be responsive but in an erratic way like changing size and leaving the background color from the blend visible to which my only solution was to min-max width the breaking points which feels kinda wrong or idk.

    also regarding the flexing part, the only way i managed to flex was to encapsulate the texts and image in 2 different boxes so i can flex them, and then on the the texts box use smaller boxes for the rest of the text/lists so i can work with them, flex them etc.. was this approach and way of thinking correct?

    since i never received any feedback on my first project i really hope that someone will help me a little bit on this one.

    thanks a lot in advance.

    stat preview card using Html/CSS

    2

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Ildrim and congrats on finishing this challenge 👏! Here are my suggestions:

    • There is way too much usage of margin and padding in your css ! for texts you can use text-align:center and text-align:left and you don't need any margin on them.
    • Give border-radius:10px and overflow:hidden to the parent element instead of giving border-radius to children separately.
    • Instead of giving margin to children, give a padding to parent so everything will align nicely.
    • for stats I think you'll be better off with something like
    <ul>
      <li>10k+<span>companies</span></li>
      <li>314 <span>templates</span></li>
      <li>12m+ <span>queries</span></li>
</ul>

    You don't need seperate ul for each single stat.

    • for centralizing your element you can use
    display: flex;
 justify-content: center;
 align-items: center;
 min-height: 100vh;

    on your body instead of giving static margin to the elements.

    • Make sure to give width:100% and then max-width:...px so your design won't break on smaller screens.
    • There is no need for p-div you can style p itself instead.

    ✅ Also I opened a pull request to your repository which will fix some of these issues

    I hope this was helpful

    Marked as helpful

    4
  • Ildikó Balogh• 80

    @liana5555

    Submitted

    I have a question in connection with the desktop version. When we click the button and the container of the share, facebook, twitter, pinterest stuff appear, how can I make it so that it will be the same place no matter the window site. Cuz, I tried to force it to be above the button and it is in the right position when the width is 1024px but if I make the window size wider it ends up going away from the button a bit horizontally. Other than that I don't have any specific question but I would be happy to receive any kind of feedback.

    Article preview component with HTML, SASS, JavaScript

    1

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Ildikó 👋 ! add position:relative to the parent element which in this case is main tag, then add position:absolute to .social-container on large screens, so .social-container will always position itself according to it's parent. Also you need to give overflow:visible to main on larger screens.

    ✅ I've already opened a pull request to your repository which will fix the issue

    SIDE NOTE : use prettier extension on your IDE to format your code

    I hope this helps

    Marked as helpful

    1
  • Yazdun• 1,310

    @Yazdun

    Posted

    Hello Ajiboso and well done 👏 ! Here are my suggestions:

    • On ACCESSIBILITY issue : each page should have at least one main tag which wraps the whole page's content and this main tag, must have a level one heading which tells what this page is about, this h1 can be hidden. your html contains main tag so just add a h1 to `main and your accessibility issues will be gone. Don't forget to generate new report though !
    • for font family, it seems like you've forgot to add comma font-family: 'Poppins'sans-serif;, make sure to change it to font-family: 'Poppins', sans-serif; so your fonts load correctly.
    • I think box shadow is too thick, I would've used something along the long with box-shadow: rgba(149, 157, 165, 0.2) 0px 8px 24px;
    • Use prettier extenstion on your IDE to format your code.

    ✅ Also I opened a pull request to your github which take care of above issues

    I hope this was helpful

    Marked as helpful

    1
  • Yazdun• 1,310

    @Yazdun

    Posted

    Hello Nyein and great job 👏 I have some suggestions on this :

    • First, Wouldn't it be cooler if button had some hover effects? 😁
    • I noticed you've been used div for form which is not best approach, best is use form tag itself and on submit you can use something like
    btn.addEventListener("click", (e) => {
  e.preventDefault();
  validateForm();
});
    • For javascript, I noticed so many if and else which was not necessary, you can write a function for that then just pass the parameters to the function, so you can totally avoid if and else repetition.

    Here I have some good resources that you maybe interested in :

    ✅ Also I opened a pull request to your solution repo which will add a function to validate your inputs so you can get rid of some of those if and else, Also I added some hover effects to your button I hope you're ok with that 😃

    SIDE NOTE: For validate function, I wanted to add a foreach loop so we wouldn't need to call the function 4 times, but I thought it would get too complicated and so different from your code, so I kept it simple a little bit. You can look into map function yourself later on.

    I hope this was helpful

    Marked as helpful

    0
  • Yazdun• 1,310

    @Yazdun

    Posted

    Hello Vishal 👋 I have one thing to add to argel's great feedback !

    instead of using p tag for your headings, use h1 to fix your ACCESSIBILITY issue

    Marked as helpful

    0
  • Guilherme Moraes• 30

    @guilherme-dm

    Submitted

    I'm curious about how you guys have handled responsivity on this one. I feel like i had to trigger a breaking point quite soon because text were starting to get very messy. If you have any suggestions, let me know 🤙

    Responsive Stats Preview Card Component

    1

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Guilherme ! Well done on this challenge. I studied your approach a little bit and here are issues I found :

    GIT ISSUE

    • Before we get to the solution itself, There is a issue with your repo. I suggest you to create a main branch and put each solution on separate branch, so it will be easier to maintain the repo and also other people can contribute to your code more easily !

    SOLUTION ISSUE

    • You don't need two media queries, first write your style for mobile and only then, write a media query for desktop.
    • Instead of span, You must use div for block level elements, for example .text-content is a block level element so you must use div instead of span.
    • for stats, You'll be better off with something like
    <ul>
      <li>10k+<span>companies</span></li>
      <li>314 <span>templates</span></li>
      <li>12m+ <span>queries</span></li>
</ul>

    instead of using div and span and heading. using ul is more efficient in this case IMO.

    • html elements are block level by default ( also this is one of the reason for why approaching mobile first is better ), so you won't need display:flex and flex-direction:column for initial style, you get the column by default.
    • Specify two font families in your styles, so if one of them is not supported by user's browser, the other one will be loaded and your design won't break.
    • Instead of .page-container, Use
    display:flex;
justify-content:center;
align-items:center;
min-height:100vh;

    on body to centralize your element.

    • Instead of giving h1 and p text-align:center;, give their parent text-align:center and all children will inherit it.
    • You have access to the initial style you write for a element in media queries, so you only need to change parts that need to be changed, for example you don't need to specify font-family in media queries, it will remain the same anyway unless you want to change it.
    • Use prettier extension on your IDE to format your code.

    ✅ Also I opened a pull request to your repo which will fix some of these issues

    I hope this was helpful

    Marked as helpful

    1
  • P
    Sara Dunlop• 220

    @Risclover

    Submitted

    I think I've done really well with this one. I wasn't sure how to do the colored image and would appreciate some correct methods for that solution. What I am really proud of is my page's responsiveness, and I would love some feedback to see if I'm doing everything correctly or not. Thanks!

    Stats Preview Card Component using HTML and CSS (w/ Flexbox)

    1

    Yazdun• 1,310

    @Yazdun

    Posted

    Hello Risclover and this looks great ! Here I have some suggestions:

    • Approach mobile first, It will make your life much easier

    • for stats, I think you'll be better off with something like :

    <ul>
      <li>10k+<span>companies</span></li>
      <li>314 <span>templates</span></li>
      <li>12m+ <span>queries</span></li>
</ul>

    In this case ul is more efficient IMO.

    Overall great job and keep it up 👏

    0