float for desktop img, img uses cover and viewport units

Community feedback

• 

  P

  AjeaS•715

  @AjeaSmithPosted about 5 years ago

  Hey Jhabari,

  Nice work on this project. It scales down very nicely, I know implementing the javascript part was not easy.

  I have noticed that there is a bit of scrolling on the front page to see the rest of the image, maybe you could try to have the banner image have a height: 100vh.

  • I also noticed you had some html issues with the form, its a minor one. if your not using the action="" attribute in your form, then its safe to remove it.

  1

  ---

  

  Jhabari Guidry•190

  @J-GuidryPosted about 5 years ago

  Thanks for the feedback. For only seeing part of the image, on mobile view, being at 70vh height and 100vw width and background-position: top center, was me trying to get the image to focus on her face as the width gets smaller, but it looks like it may not work well. Removing top center looks much better but the image will still cut off somewhat at the sides. 100vh on mobile views makes it zoom in far more cutting much of the image. I am not sure vh will solve this completely, at least not without a lot more media queries. Perhaps testing screen ratio queries may be needed.

  As for desktop view, I do have it at 100vh with 50vw floated with the rest of the content on the other side. It works except for ipad pro's 1024 x 1366 resolution which cuts off half the image. Looks like high heights to lower widths break the design depending on disparity.

  0
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted about 5 years ago

  Jhabari, great work on this challenge! Just a quick heads up that uppercasing text in your HTML will lead to screen reader software reading the content out letter-by-letter. So, in your h1, I'd recommend lowercasing the text and then using text-transform: uppercase; in your HTML to match up to the design.

  0