Skip to content
  • Unlock Pro
  • Log in with GitHub
Profile
OverviewSolutions
10
Comments
10
P
David Lemvigh
@dlemvigh

All comments

  • MichalTrubini•1,220
    @MichalTrubini
    Submitted over 3 years ago

    Meet landing page, mobile first, SASS and BEM notation

    1
    P
    David Lemvigh•200
    @dlemvigh
    Posted 5 months ago

    The visual

    • There is a scrollbar near the faces
    • The purple buttons are a bit brighter in color that design

    Generally really good solution, nice and responsive on all screen sizes.

    The code

    • Nice semantic html
    • Well organized css classes
    • An elegant solution to the desktop faces. The design shows the faces closer to the edges. But with your solution that could easily be changed by adding justify-content: space-between to the .banner__container. (I also like your design better than the figma design on that front)
    • I think the cause of the scrollbar is the overflow-x: hidden, which I don't understand the reason for. Removing it removes the scrollbar. (It is not the first time I've see overflow-x/overflow-y having unexpected interactions, but I don't know/remember why)

    Note: I tested in Chrome on Windows 11, if that helps with debugging the scrollbar.

  • P
    Ishmael Sunday•250
    @okayishmael
    Submitted 5 months ago
    What are you most proud of, and what would you do differently next time?

    Mixing grid and flexbox to achieve my goal.

    What challenges did you encounter, and how did you overcome them?

    The design indicated 13px font size.16px/ 1rem is what I used for readability. I some people can see to well. I am just advocating for users

    What specific areas of your project would you like help with?

    My is seems a little bigger.

    Responsive testimonials grid section | htm5, css3

    1
    P
    David Lemvigh•200
    @dlemvigh
    Posted 5 months ago

    The Visuals

    • Looks really good
    • Avatar outlines a bit different than design, but looks good
    • Nice 4, 3, and 1 column layouts
    • A bit extra body margin on mobile layout compared to design, making the content very squezed on when get below 400px
    • Missing "watermark" on purple card (also found it annoying to add myself without getting it on top of the text, but find a nice way by setting background-image on the card).

    Visually really good, only small notes

    The Code

    • The is an attempt at css grid, but then a lot of flexbox inside doing most of the work (grid-template-columns: repeat(6); - repeat is missing its second parameter so gets ignored)
    • A bit of grid and flexbox nesting, would have been nice to have been done with a single grid. Maybe using order to put the "kira" card on the bottom.
    • Well structured html
    • Nice use of css variables

    Flexbox usage really good. Grid usage could have been better. But in general really nice solution

  • P
    Will Cooley•200
    @CooleyWC
    Submitted 5 months ago
    What are you most proud of, and what would you do differently next time?

    This was my first time using Sass for CSS

    What specific areas of your project would you like help with?

    I know my box-shadow for the cards isn't quite like the Figma design - I could use help with this.

    Four Card Feature

    2
    P
    David Lemvigh•200
    @dlemvigh
    Posted 5 months ago

    The visual

    Looks great, minor discrepancies from the design, but it feels more like a stylistic choises than mistakes.

    Nice use of CSS grid

    The code

    Nice clean HTML, the only petty comment is that you have multiple h1 headings which might not be the best for accessibility, but I could be wrong on that.

    The scss files are generally well structured. There is a bit of .hero styling that has snuck into _typography.scss which is confusion, and probably should be in _hero.scss.

    Marked as helpful
  • nada123432h•20
    @nada123432h
    Submitted 6 months ago

    https://nada123432h.github.io/mentor_1/

    1
    P
    David Lemvigh•200
    @dlemvigh
    Posted 5 months ago

    ** The visuals **

    • many visual differences from the design ** background color ** fonts ** image size
    • the layout kinda work on exactly the breakpoints 320px, but looks pretty broken on most other width < 460px

    ** The code **

    • A lot of inline-styles
    • Mix of classes and inline-styles
    • Does use flexbox and media queries for layout
  • Levi•110
    @Levi271997
    Submitted 7 months ago

    Dictionary App with React and Sass

    #react#sass/scss
    1
    P
    David Lemvigh•200
    @dlemvigh
    Posted 5 months ago

    Visual

    The good

    • Looks great, really well done.
    • Love that you have added a loading state that wasn't in the design.
    • Audio playback worked really well

    The bad

    • font switcher is a bit off, options are light in dark-mode and dark in light-mode
    • keyboard accessibility are missing a few things ** dark mode can be toggled, but there is no focus outline ** font switcher cannot be accessed using keyboard navigation (tabbing throught the page). *** could potentially be fixed by adding tab-index=0, focus styling, and keydown event handler

    Code

    • 400 lines of react in one file is a lot, would be better if it was split up into at least a couple of separate files
    • looks like it is a create-react-app which has been deprecated and not updated since 2022

    CSS

    The good

    • I like the of variables and the mixings
    • especially the $typography-styles, letting you mimic the figma usage of defined typographies

    The bad

    • 700 lines of css in a single files is a lot.
    • a lot of nesting in the rules. This causes increased specificity which can make extending the classes in the future difficult, without !important and other hacks.
    • I'll be honest I got overwhealmed by everything being in one big filed, and haven't reviewed it that well.
  • davistar21•180
    @davistar21
    Submitted 6 months ago

    Mortgage Calculator using HTML, CSS and Vanilla JS

    1
    P
    David Lemvigh•200
    @dlemvigh
    Posted 6 months ago

    the good

    • I am impressed at that the page is implemented in vanilla js
    • well organized html (minor mismatch in label for mixing kebab-case and camelCase)

    the bad

    • focus styling a bit off for year and interest rate, looks different than the focus styling for amount
    • error styling a bit off. The input addon ($ , year , %) does not have red background.
    • radio inputs have no focus styling, so keyboard users have no outline to show focused element.
    • no validation messages are shown ("this field is required")
    • clear doesn't clear validation state. When clicking clear all, I would have expected the validate state to also be cleared.
    • "clear all" not keyboard accessible (could be fixed with tabIndex="0") or making it a button with styling

    conclusion

    A solution that fulfills the technical requirements, with semantic HTML. There are many deviations from the design in terms of styling.

    Marked as helpful
  • Khai•80
    @khlv2219
    Submitted 6 months ago
    What are you most proud of, and what would you do differently next time?

    I was very proud of recreating this design without any further help of external resources.

    What challenges did you encounter, and how did you overcome them?

    My biggest challenge was dealing with position/display properties of the table. I tried very long different properties and finally got the solution I wanted.

    Recipe page challenge with basic html and css

    1
    P
    David Lemvigh•200
    @dlemvigh
    Posted 6 months ago

    Paise: You really nailed to desktop design (way more than I could be bothered myself).

    Comment: The mobile design seem to be missing, where the margin and border-radius is removed (among other things).

    Comment: Your use of css variables is pretty good. Except the font-weights --fw-1-400: 400; with this naming convension it ends up being a longer way of writing 400, and if you wanted to change the weights in the future you would end up changing the names to match.

  • Ifeoluranmi•30
    @Ifeoluranmi
    Submitted 6 months ago
    What are you most proud of, and what would you do differently next time?

    It was nice working on this challenge as i attempt the Idea Test being suggested by Frontend which is making visitors to navigate links via keyboard only, it's quite fun trying out the challenges as it put my knowledge to test, learning and relearning stuffs that i might have forgotten.

    Social links profile using HTML and CSS

    1
    P
    David Lemvigh•200
    @dlemvigh
    Posted 6 months ago

    The HTML is nice and accessible, but the design have few missing a few things. The links does have a focus state, but not the one from the design. In your case you could have used li:focus-within { } to style the list items if the links inside have focus.

    There is no hover state on the links.

    There are also some side padding on the card missing.

    Nice touch with the ease-in-out transitions

  • Macdonald Nkhata•90
    @MacNkhata
    Submitted 6 months ago
    What are you most proud of, and what would you do differently next time?

    I am proud that I reduced the use of divs as pointed out in my last submission. This has helped to make my html files concise and easy to read.

    What challenges did you encounter, and how did you overcome them?

    I had troubles using the static fonts provided.

    What specific areas of your project would you like help with?

    I dont have for now

    Blog Preview Card

    1
    P
    David Lemvigh•200
    @dlemvigh
    Posted 6 months ago

    Very clean solution. No notes

  • TCHINGUÉ MAKAMTÉ Patricia Kévine•100
    @Pattykev
    Submitted 6 months ago
    What are you most proud of, and what would you do differently next time? What I learned
    • How to auto values om margin properties : In CSS, the margin property is used to create space around elements. When you set the margin to auto, it allows the browser to automatically calculate the margin space. This is particularly useful for centering block-level elements within their container.

    Here's what margin: auto; typically means:

    1. Horizontal Centering: When applied to a block element (like a div), setting the left and right margins to auto allows the element to occupy the available horizontal space equally on both sides, effectively centering it within its parent element.

      .centered {
          width: 50%; /* or any other width */
          margin: 0 auto; /* top/bottom margin 0, left/right margin auto */
      }
      
    2. Vertical Margins: The margin: auto; setting does not have the same effect for vertical margins (top and bottom) unless certain conditions are met (like using flexbox or grid layouts).

    3. Flexbox and Grid: In flexbox or grid layouts, margin: auto; can also be used to distribute space between items or to push items to the edges of their container.

    In summary, using margin: auto; helps in controlling the layout and positioning of elements within a webpage, particularly for centering elements horizontally.

    • Remembered how to change local repository url git remote set-url origin <put the new url there>
    • How to insert an image with markdown

    ![<alternative tex>t](<image url>)

    What specific areas of your project would you like help with?

    In CSS learning.

    qr-code-component with HTML5 and CSS3

    2
    P
    David Lemvigh•200
    @dlemvigh
    Posted 6 months ago

    Generally a nice and clean approach.

    On the .container you set margin: 9rem auto;. As you correctly stated the sets the auto used for both left and right margin centering the content. The css shorthand you use also use 9rem for both top and bottom margin, where the bottom margin might not be intentional (on mobile it could add a scrollbar due to the short height of the page).

    For the container you also define a fixed height for the container itself height: 32rem;. I would suggest just removing the height, and let the content inside determine the height of the container. That would make your solution more robust to future changes. In your case you would have to add some bottom margin to the last element to get the proper padding.

    The last point would be to maybe set padding on the .container instead of having left/right margin on all its child elements.

    Marked as helpful
Frontend Mentor logo

Stay up to datewith new challenges, featured solutions, selected articles, and our latest news

Frontend Mentor

  • Unlock Pro
  • Contact us
  • FAQs
  • Become a partner

Explore

  • Learning paths
  • Challenges
  • Solutions
  • Articles

Community

  • Discord
  • Guidelines

For companies

  • Hire developers
  • Train developers
© Frontend Mentor 2019 - 2025
  • Terms
  • Cookie Policy
  • Privacy Policy
  • License

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub

Oops! 😬

You need to be logged in before you can do that.

Log in with GitHub