Skip to content
OverviewSolutions
42
Comments
21
madhaus
@festsnusa

All comments

  • Elizabeth Sotomayor•230
    @elizabethrsotomayorSubmitted almost 2 years ago

    I wasn't sure if I was meant to use async/await for the fetch API call. I ran into an issue on larger screens where I couldn't center the divider. The box-shadow when hovering the button is a little bit offset since the shadow is larger on the bottom.

    Advice generator app made with HTML/CSS/JavaScript

    1
    madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge.

    Here's my feedback:

    • to center your divider, you can use "left: 50%" and "transform: translateX(-50%)";
    • when clicking more than once within 2 seconds, the second click is ignored. you can set timeOut with 2 seconds;
    • importing google fonts instead of downloading fonts is not a good practice, because anything could happen — fail to connect to link or the actual link may become unavailable. to download fonts, you can use "google-webfonts-helper";
    • i think wrapping "main — wrapper — card" looks kinda complex. you can simplify it to "main class='card'";
    • i don't think putting 2 images inside .dividers is a good idea. you can change the image via media query;
    • putting div inside a is not a good idea. only "inline-block" elements are recommended. plus, your whole div section will act as a link;
    • alts are empty;
    • in CSS, h1 declared twice;
    • for media queries, you can only set for desktop resolution;
    • your box-shadow looks okay;

    Hope you're gonna find my feedback useful. Happy coding.

    Marked as helpful
    1
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge.

    Here's my feedback:

    • your page is not responsive/adaptive. when i resized my page to mobile resolution, everything looks messy. though you have a media query. you can add "width: 100%" to your component;
    • .DS_Store - i think this file is unnecessary;
    • to make your html code look more concise, you can try to use Pug and add mixin. Also, instead of pure CSS, you can try SCSS.

    Hope you found my feedback useful. Happy coding.

    Marked as helpful
    0
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge.

    Here's my feedback:

    • to place your main component in the center, you can use grid: display: grid; min-height: 100vh; place-content: center;
    • try to place your styling into the CSS/SCSS file, not inside HTML file;
    • importing google fonts is not a good practice. to download fonts to your hub, you can use "google-webfonts-helper";
    • you can try to use SCSS instead of pure CSS, because you'll be able to use mixins and include nestings;
    • i guess, "Learn More" should have "cursor: pointer";
    • in your JS file, instead of adding styles, you can toggle classes. and instead of using "else", you can use "return".

    Hope you found my feedback useful. Happy coding.

    Marked as helpful
    0
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge.

    Here's my feedback:

    • there's an empty div in your html file;
    • importing fonts instead of downloading them is not a good practice, because google fonts link may not be available;
    • in SCSS, putting "&" inside selector like this "& a.overlay" is not necessary;
    • to make your SCSS code look more concise, you can try mixins.

    Hope you found my feedback useful. Happy coding.

    Marked as helpful
    1
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge.

    I'm quite impressed by your page's design with transitions and animations. So, I'm gonna skip to advices to how to make your code look more concise:

    • importing fonts from Google Fonts are not recommended, because link may change or may become unavailable. So, instead of importing, I recommend you to download it. you can use "google-webfonts-helper";
    • instead of px, you should use rem;
    • in your scss code, i've noticed that you constantly put the whole selectors inside selector. perhaps, you should use BEM;
    • you can switch to Pug, include mixins and separate your page into components.

    P.S.: I've noticed a bug:

    • when u open nav menu and switch to desktop resolution, your ul will look kinda messy.

    Hope you find my feedback useful. Happy coding!

    Marked as helpful
    0
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge.

    Just a few bugs I've noticed while testing your page:

    • your paragraph's font in .topcontainer is different. perhaps, you should apply font-family to your body;
    • i've tested mobile screen on Chrome and everything looks okay. but when i switched to Opera, elements in .title collide w/each other;
    • instead of px you should use rem.

    To make your code look more concise, I recommend you to use Pug & SCSS. So, it's possible to minimize your code using mixins and loops: https://pugjs.org/language/mixins.html

    Hope you find my feedback useful. Happy coding!

    0
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge.

    Here are some bugs I've noticed:

    • when I resized my screen for mobile devices, your background image in header didn't change;
    • your filter works incorrectly. for example, i picked "Frontend" and only 6 cards left (as planned). But when I add "CSS", 6 cards remain. in my opinion, in this case, only 2 cards should remain visible;
    • not quite sure if implementing React for this challenge was necessary. but when I refresh the page, the filter is lost. Perhaps, you should save user's picks to localeStorage.

    Hope you find my feedback useful. Happy coding!

    Marked as helpful
    1
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge!

    I've noticed that you didn't use media queries. So, when you adjust your screen to mobile devices, everything smashes. You can set "width: 100%" to your components.

    Plus, I'm not a big fan when someone uses the same element over and over again. You can add some mixins via Pug. Here's an example: https://pugjs.org/language/mixins.html. Also, instead of using pure CSS, you can improve your code using SASS.

    Hope you find my feedback useful. Happy coding!

    Marked as helpful
    0
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for completing this challenge.

    I'm not quite sure if implementing React was necessary, because your styling component written in JS is kinda huge. You could minimize it using mixins in sass.

    Also, I've noticed that:

    • the whole item is unclickable. I guess if you click on an item, notification counter should decrease by 1 and an item itself should change its color;
    • when you cllick on "mark as read", notification counter should be invisible;
    • perhaps profile pictures and photo with chess should act as if there are links.

    Hope you find my feedback useful. Happy coding.

    Marked as helpful
    1
  • himanshukrmr•50
    @himanshukrmrSubmitted almost 2 years ago

    Hi Community,

    Tried to make this multi step form using HTML, SCSS and JQuery. Almost all functionality is done but I was stuck in some. Comments are welcome for improvement.

    Thanks

    multi-step form

    1
    madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! Congratulations for finishing this challenge! I've tested your page and found some bugs:

    • I found weird that arrows for phone number are available. Plus, you should limit your phone number length. Perhaps you should use imask.js for phone formats;
    • when I filled the name input incorrectly, and then edited it to the correct way, the red border is still there;
    • when I step forward and then go back to the step 1, the red borders are still there;
    • on the step 2, i'm able to choose several plans at one time. that's incorrect;
    • on the step 4, when i don't choose plan "Arcade", my choice is still "Arcade".

    Hope you find my feedback useful. Happy coding!

    Marked as helpful
    0
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Hi there! I see that your whole page is not responsive, not only nav section. Here's my advice to how to add menu section:

    • add burger as image;
    • make your nav section flexible and add justify-content: space-between. So, logo on the left, burger on the right;
    • after burger, create ul with items "about", "careers", "events", "products", "support";
    • when you click on burger, the ul should cover all the page. Solution: via JS you can addEventListener when you click your burger and toggle active classes. For example:
    • header__burger_active - to transform your menu icon to close icon. scss code looks sth like this: &_active { position: fixed; right: 0; top: 1.4rem; padding-right: 1rem; content: url('./images/icon-close.svg'); z-index: 3; }
    • header__list_active - to make your ul cover all the page. scss code looks like this: &_active { display: block; position: fixed; padding: 10rem 3rem; top: 0; bottom: 0; right: 0; width: 100%; background-color: $black; z-index: 2; }
    • header__logo_active - to make logo appear with active ul: &_active { position: fixed; top: 1rem; }

    Hope you find my advice useful. Happy coding!

    Marked as helpful
    1
  • madhaus•1,040
    @festsnusaPosted almost 2 years ago

    Congratulations for completing this challenge!

    Here are some advices to how to improve your code:

    • I don't think main-wrapper class is necessary;
    • Also, when I resize my screen to mobile devices, everything clashes. You should add some media queries and set everything to width: 100%;
    • I guess, logo should be clickable and buttons should have pointers. Items in .top-left should have pointers too and act as if there are links.

    I hope you found my feedback useful. Happy coding!

    0