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
OverviewSolutions26Comments87

All comments

  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi! Martins, great job on finishing this one, it looks good.

    some suggestion to improve your code and responsiveness:

    I did some changes in dev tools:

    • You need to change the grid-template-column 1fr 1fr value into fraction here the link to understand more about grid fraction. grid fraction.
    • you need to set your main width:100% and then add max-width: 876px so that it will response when the viewport is 876
    • add another 100% to your .image class background-size: 100% into background-size: 100% 100%
    • Use unordered list <ul> for .subtitle. HTML lists are used to present list of information in well formed and semantic way.

    Overall you did well, Hope this help!

    cheers, benjo

    Marked as helpful

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi GuidiUZ, great job on finishing this one, it looks good and response rather well.

    some suggestion to improve your code;

    • You need to change your <input /> button into <button> tag. The difference is that <button> can have content, whereas <input> cannot (it is a null element).
    • Instead of using <img> elements to wrapping your .logo, .logo-empress, .socials and .footer-nav you should use anchor element <a> because this element is going to transfer user somwhere.
    • Considering adding a alt attribute on your images. Read more why we need to add alt

    Good luck with that, have fun coding!

    cheers, benjo

    Marked as helpful

    1
  • Esteban Salas• 80

    @Geo0510

    Submitted

    Hi everyone,

    I've completed profile card component challenge. Any feedback or suggestion will be very considered and helpful.

    Thank you all.

    profile-card-component-main

    2

    Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi Geo0510, great job on finishing this one, it looks good.

    some suggestion :

    • I've also been at this place where you want to use the <img> tags instead of using a background image.(feels like it would be more flexible) But I realized, the custom changes you can make with background images are way more than <img> tags...so I would suggest you to use them.
     background-image: url('images/bg-pattern-card.svg');
    • You need to change the width of .card_container into width: 100%, and add a max-width: 400px;, so that it will shrink when the viewport is 400
    • You also need to change that body background into background images. if you find it tough you can always check at my solution here: https://www.frontendmentor.io/solutions/html-csssass-hbRxlzWFH

    Goodluck and keepcoding.

    cheers, benjo

    Marked as helpful

    0
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi RTX3070, great job on finishing this one, it looks good and response rather well.

    some suggestion to improve your code.

    • Heading(h1-h6) levels should only increase by one, because you use h5 heading, it gives you a warning, use h3 instead you cannot use h5 without h3 and h4.
    • Your h1 tag should be at the top of the page content (above any other heading tags in the page code).
    • I think you put media queries early, use the min-width: 1025px or 768px instead.

    Overall you did well, hope it helps

    cheers, benjo

    Marked as helpful

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi! Sai Pradeepti, you did a great job on finishing this one, it looks good and response rather well.

    some suggestion to improve your code.

    • I think you need to add more route path for search only <Route path='/Search' element={<SearchResults />} /> because the page on your website contradict when searching and filtering, user can't filter the when the user still searching.
    • When searching in dark mode, dark color is only half on the screen viewport, you need add a min-height your your .app and put the background-color on class .app so that the whole viewport will have a dark color.

    Overall you did well. Keep coding and happy coding too!

    cheers, benjo

    Marked as helpful

    1
  • Olisaemeka• 260

    @olisa187

    Submitted

    Hi Front-end mentor community!

    Finished my third project on frontendmentor.io, I would really appreciate and love if I can get some feedback from the community concerning my flaws and strengths.

    Huddle Landing Page

    #accessibility

    1

    Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi Olisaemeka, great job on finishing this one, it looks good and response rather well.

    some suggestion to improve your code:

    • You need to wrap your content class into main tag so that it will fix the accessibility issues.
    • you need to add a aria-label on social icon. aria-label simply used to provide a text alternative to an element that has no visible text on the screen.
    • and also need to remove some em unit of the page, use EM where you have to make more scaling than the root font size and use. And use REM where you need value according to the root there you can use REM units

    Keep coding and good luck

    cheers, benjo

    Marked as helpful

    0
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi Nasim Reja

    You did a great job on this one. It looks good and response rather well.

    Some suggestion to fix your A11y or ACCESSIBILITY ISSUES to ensure that all link names are accessible you need to add the aria-label on your logo defining where the link would take them. Eg. <a href="#section1" style="width: 17rem;" aria-label="Bookmark - Home Page">

    As for the email validation you can add this on your script file.

    form.addEventListerner('submit', function(e) {
    e.preventDefault();
    const email = document.querySelector('#email');
   let re = /^([a-z\d\.-]+)@([a-z\d-]+)\.([a-z]{2,8})(\.[a-z]{2,8})?$/;

   if (re.test(email.value)) { // Will check if there a value of the re variable
      success.classList.add('success');
   } else {
     success.classList.remove('success');
   }
})

    Keep coding and Good Luck!

    1
  • Stefan• 620

    @stfnpczk

    Submitted

    In this challenge I experimented quite a bit w grid layout. Making the image grid responsive was tougher than I thought, so I'd really appreciate any suggestions how to better go about it.

    Thank you :)

    Two-Page Art Gallery Website using SCSS and Grid

    3

    Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hey Stefan

    I really like your solution 😄.

    I don't have any suggestion as your solution is already great.

    Marked as helpful

    0
  • Shameer Kamaludeen• 440

    @shameerkamaludeen

    Submitted

    Hi everyone, I would like to ask a couple of questions that came to mind during the development of this solution.

    • Does cards should be fixed or responsive always
    • Here used 'mix-blend-mode' property on the image to get the effect of the card image, what is the best suggestion.
    • What do you prefer to use in the solution, only flexbox, grid or something else?

    Responsive Stats preview card component usig flexbox

    #accessibility

    2

    Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hey @shameerkamaludeen

    Great job on finishing this one. It looks good and response rather well.

    • It looks response in my screen😅
    • I think 'mix-blend-mode' is also one of the best solution as you don't need to add another CSS property, one of suggestion is using pseudo-elements like ::before or ::after.
    • You can both use flex or grid as this two is great for responsiveness. If designs is two-dimensional use grid, if designs is one dimensional use flexbox instead. You need to decide to yourself of what you want to use, as these two will make your website responsive.

    Some suggestion:

    • Use unordered list <ul> for .stats-wrapper. HTML lists are used to present list of information in well formed and semantic way.

    • Remove margin: 8.5em 0; on your .stats-card. Because you already declare the main tag

       display: flex;
   align-items: center;
   justify-content: center;

    the child is already centered as the margin don't have any use at all.

    Aside from those everything is great👍. (Sorry for the bad English as I'm still practicing😅)

    Keep coding and Good luck

    cheers, Benjo

    Marked as helpful

    2
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi MiguelHG2351

    Great job on finishing this one and It looks good and response rather well

    Some concern about your accessibility and html issue.

    • Document should have one main landmark. Your body tag should only contain these 3 landmark element. It will be better if the structure of your html page is like this, as you are using HTML5 syntax.
    <header />
<main />
<footer />
    • You need h1-h6 to your section tag. If you don't need one use the visually-hidden instead.

    Some suggestion:

    • Change the alt attribute of your .header-logo. "logo" is not descriptive, and since you wrap your image with anchor tag and it direct to home page maybe change alternative text to "Bookmark - home page".

    • Instead of using <img> elements to wrapping your .footer-social-media you should use anchor element <a> because this element is going to transfer user somewhere. Ex: <a href="#"> the image of social media </a>

    • I don't think you need to add another div for your mobile menu you can manipulate it using JavaScript. I suggest you to delete those and try manipulating .desktop-nav-container into mobile. You can do it😃.

    Keep Coding and Goodluck

    cheers, Benjo

    Marked as helpful

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    HI @ayushv45

    Congrats on finishing this challenge.

    • It will better to learn flexbox and grid. It makes our life easier to design and build responsive web pages

    Some suggestion:

    • Use unordered list <ul> for number status. HTML lists are used to present list of information in well formed and semantic way.

    Keep coding and happy coding too!

    Marked as helpful

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hello @SimoniacJewel88

    Congrats on finishing another challenge! 🎉 Your solution looks good

    Some suggestion :

    • Use unordered list <ul> for .stats. HTML lists are used to present list of information in well formed and semantic way.
    • And on your tarjeta-descripcion section you should remove the padding left and right because the elements inside of it is overflowing in 1026px screen.

    Keep coding and happy coding too!

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hello, Shawky! 👋

    Congrats on finishing another challenge! 🎉 Your solution looks very good and also responds well.

    If you are having problems in the features section you can check my work here the link

    Here some suggestion:

    • All page content should be contained by landmarks. It will better if the structure of your html page is like this
    <header>
  <nav></nav>
</header>
<main>
<section>
</main>
<footer></header>

    _ You need to add the navigation link. Because this element is going to transfer user somewhere. <li><a>tag here</li> and also best practices.

    On Footer

    • Instead of using <img> elements to wrapping your social container you should use anchor element <a> because this element is going to transfer user somewhere. Eg: <a href="#">Social icon here </a>

    Aside from those everything looks good to me. Keep coding and happy coding too!

    0
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hey @developython14 Nice job on finishing this one

    Some suggestion:

    • All page content should be contained by landmarks. It will better if the structure of your html file is like this.
    <header></header>
<main></main>
<footer></footer>
    • You forgot to add the base or reset elements
    html { 
   font-size: 100%;
   box-sizing: border-box;
}
* {
   box-sizing: border-box;
}
    • You can check my solution here, if you want to upgrade your page Solution

    Good luck

    Marked as helpful

    0
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hey @Alexinzunzav Great job on finishing this one, It looks good

    Some suggestion:

    • Remove the height: 100vh or just add the min-height: 100vh on the body tag. Because this will only limit the body to have a height 100% of the remaining viewport or screen.
    • Also avoid using width: 100vw in a big container like body or main. Because it will only add a horizontal scroll bar.
    • Also must add the cursor: pointer on button, to inform the user that this is clickable and not just for design.
    • Invalid property on * selector border: border-box; box-sizing instead

    Aside from those everything is good, keep coding and happy coding too!

    Marked as helpful

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Great work @1Hanif1, Everything looks great and response very well. I really like the animation you add on this challenge.

    Small suggestion I would recommend removing your commented code, it's not a big deal but it's the kind of thing that makes your code look a bit unprofessional, so it's probably better to remove it.

    Good luck and keep coding

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hey @ahemadShaikh. Great job on finishing this one. It looks good and response rather well.

    Some suggestion:

    • All page content should be contained by landmarks. It will better if the structure of your html page is like this
    <header> </header>
<main></main>
<footer></header>
    • Don't forget to add a alternative text on your <img> tag when user having a internet problem. the logo must have a alt="manage logo" something like that, and also add the alt on the person image with there name.
    • Every section need a headings h1-h6
    • The footer link also should have anchor element <a>. Because this element is also going to transfer user somewhere. <a href="#"><img> tag here</a>
    • You must add the e.preventDefault() on your form to prevent the page from refreshing

    Good luck and keep coding

    Marked as helpful

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hey @faruking, great work on finishing this one. I really like your solution.

    Some suggestion:

    • You forgot to add the base or simply put a reset, you must add the
       font-size: 100%;
   box-sizing:  border-box;
}

*, *:: before, *::after {
    box-sizing: inherit;
    padding: 0;
    margin: 0;
}
    • If you want a main content of your page must add the <main> tag for a big container like this one.
    • If you want to center you container add this on your body tag.
    min-height: 100vh;
 display: flex;
justify-content: center;
align-items: center;

    so that the your main container will center no matter what screen size.

    • I'm also like you before I'm using media queries in max-width: 375px and that completely is wrong you should change it to 768px.

    Aside from those everything looks good to me keep coding and happy coding too.

    Marked as helpful

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hello!👋 @menoo20

    Great job on finishing this one. It looks good and response very well.

    • It seems the only need to change is the <img> tag because It needs and alternative text when user having a internet problem.

    • To make organize you file clean and organize you must learn how to use the 7-1 sass architecture. Here's the link

    Good luck and keep coding!

    Marked as helpful

    0
  • dominiquemc• 90

    @dominiquemc

    Submitted

    This was very challenging but I believe my end result is close to the solution. I would love to know all changes I could've made or added. The media queries were probably the hardest. On mobile devices, I believe my media query looks OK but for ipads, tablets, etc. the image looks very big. I wasn't sure how to adjust this part without it impacting the entire media query.

    Stats Preview Card Challenge

    1

    Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hey @dominiquemc Great job on finishing this one, It looks good and response very well.

    Some suggestion:

    • If you want a main for the main content on your page you should use the <main> tag instead. -I've also been at this place where you want to use the <img> tags instead of using a background image.(feels like it would be more flexible) But I realized, the custom changes you can make with background images are way more than <img> tags...so I would suggest you to use them.

    Good luck and keep coding

    Marked as helpful

    0
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hey @GiaVoNgu

    I like your solution. There's only something need to remove and change to make looks more good

    Some suggestion:

    • The wrapper should change to <main>. All element that has content are inside their respective landmark elements.
    • You must remove the width: 100vw; your wrapper class. Because this will only add the horizontal scroll bar. -It will better if you change the media queries to 768px.

    Keep coding and Good luck!

    Marked as helpful

    1
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hello Shahd mhm 👋

    Even with complete beginner you still did a good job on this one.

    Some suggestion:

    • You must remove the transform transform: translateX(10%); on you body tag. This will only add the extra horizontal scrollbar on the small screen like me😅.
    • No need to add position relative and use left right for center the text text-align:center is already enough.
    • You might want to study more about flexbox😃.
    0
  • Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hello Fernando 👋

    Everything looks good and I like you solution. Much better than me when I did this challenge😅.

    My only small suggestion is:

    • Instead of using <div> elements to wrapping your svg logos you should use anchor element <a> because this element is going to transfer user somewhere.

    Keep coding and happy coding too!

    Marked as helpful

    1
  • sydpeay• 40

    @sydpeay

    Submitted

    I know this isn't perfect (for example I completely omitted the active states), but this is the most complex thing I've ever coded! My main question right now is how to keep your code organized. I think my classes and ids are a bit inefficient, and I think I would be able to do a better job if that was more efficient.

    Responsive Order Summary Component using Flexbox

    1

    Benjo Quilario• 1,810

    @benjoquilario

    Posted

    Hi Sydpeay, Great job on finishing this one. Everything looks good and response rather well.

    As for your question You must study the CSS Specificity. Here is the source CSS Specifity. This way you will not be confused on what selector you will going to use.

    Some Suggestion:

    • You must change the <div="container"> into <main> -I've also been at this place where you want to use the <img> tags instead of using a background image.(feels like it would be more flexible)

    But I realized, the custom changes you can make with background images are way more than <img> tags...so I would suggest you to use them.

    • If you want to use the <img> tag you do not forget add the alt attribute, so the image have a alternative text for an area.

    • Since the proceed to payment is some kind of action you must use the button on proceed and cancel.

    Keep coding and happy coding too!

    1