Skip to content
OverviewSolutions
26
Comments
87
Benjo Quilario
@benjoquilario

All comments

  • Benjo Quilario•1,810
    @benjoquilarioPosted over 2 years ago

    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
    @benjoquilarioPosted over 2 years ago

    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
    @Geo0510Submitted over 2 years ago

    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
    @benjoquilarioPosted over 2 years ago

    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
    @benjoquilarioPosted over 2 years ago

    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
    @benjoquilarioPosted over 2 years ago

    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
    @olisa187Submitted over 2 years ago

    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

    1
    Benjo Quilario•1,810
    @benjoquilarioPosted over 2 years ago

    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
    @benjoquilarioPosted almost 3 years ago

    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•730
    @stfnpczkSubmitted almost 3 years ago

    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
    @benjoquilarioPosted almost 3 years ago

    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
    @shameerkamaludeenSubmitted almost 3 years ago

    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

    2
    Benjo Quilario•1,810
    @benjoquilarioPosted almost 3 years ago

    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
    @benjoquilarioPosted almost 3 years ago

    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
    @benjoquilarioPosted almost 3 years ago

    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
    @benjoquilarioPosted almost 3 years ago

    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