Skip to content
OverviewSolutions
12
Comments
6

Jimmy Sweeney

@sweenejpSt. Paul, MN350 points

I'm a front end web developer working remotely from the Twin Cities, MN. I started learning web development on my own in 2019. Frontendmentor.io was easily the most valuable tool I discovered during my web development learning journey. For fun, I love reading sci-fi and playing volleyball!

I’m currently learning...

Front end frameworks beyond React.

Latest solutions

View more solutions

Latest comments

  • Jimmy Sweeney•350
    @sweenejpPosted over 3 years ago

    Looks good Marley. One critique I have - could you put the infoUnavailable method within your loadUser method? That way you wouldn't have to put infoUnavailable in your JSX. That way you would have all of the data ready to be accessed when you render the component. Probably not a big deal, but I think it would make it a little cleaner.

    Marked as helpful
    0
  • Jimmy Sweeney•350
    @sweenejpPosted over 3 years ago

    Hi Gregg,

    Your solution looks pretty close to the design so good work.

    One thing I notice right away is that you could have all your media queries at the end - that way you would just have two media query lines and then put all of the classes that need to update at different sizes within those media queries (instead of having a media query on all of your classes). That might just be a matter of preference though.

    Another solution to loading different image sizes is to create a div with a background-image: url(file-path.jpg) and then update the file path with media queries.

    .info__img-wrapper--brand {
transform: translateY(13px); 
}
    • don't know that this line is necessary.

    I could be wrong but it doesn't seem that overflow is doing anything. Looks like you achieved the effect of getting images to be off screen with flex-wrap: no-wrap with one of the images and a negative margin with the image. Maybe there was a way to be more consistent here and use no-wrap to achieve the effect for both images?

    Keep up the good work!

    2
  • Jimmy Sweeney•350
    @sweenejpPosted over 3 years ago

    Hi Nicolas,

    Looks like you need to adjust your media queries. Typically you don't want any side scrolling. The layout breaks at around 1300px. Personally I think it's better to set media queries based on when your layout breaks - not based on typical sizes of screens.

    You probably just need one media query here if you take a mobile first approach. Create the mobile layout, then set a min-width media query for when you want to transition to the desktop layout (somewhere around 1300px probably). Then make the necessary layout adjustments under that media query (flex-direction: row;, etc, etc).

    Marked as helpful
    0
  • Jimmy Sweeney•350
    @sweenejpPosted over 3 years ago

    Looks good to me. One thing I noticed is that the social media icons don't change color on hover.

    You could also try keeping the text within column 2 from condensing when you resize the window.

    Marked as helpful
    0
  • Jimmy Sweeney•350
    @sweenejpPosted almost 4 years ago

    Nice design! You really matched the solution well.

    I took a little different approach to the validation. I left on the required attribute because I think this keeps the error message from showing up until the user hits the submit button. Then I added an eventlistener with an event type of "invalid". Got help with this from this blog post here: https://daverupert.com/2017/11/happier-html5-forms/

    You can check out my solution here if you'd like: https://github.com/sweenejp/base-apparel-coming-soon-page

    Marked as helpful
    0
  • Jimmy Sweeney•350
    @sweenejpPosted over 4 years ago

    One small detail is that I think the card has a really subtle shadow effect. Other than that, looks great!

    I like how you used grid to set up the layout of the card. Works well.

    2