Rohit Deshpande

Latest comments

• 

  Benevolent•490

  @bene-volentSubmitted almost 2 years ago

  

  Profile Card Component with BEM

  #bem

  1

  10
  

  Rohit Deshpande•870

  @rohitd99Posted almost 2 years ago

  Hi Benevolent

  Congrats on completing the challenge.

  I'd like to suggest a few changes to your solution,

  1. For the attribution footer would be better suited than aside, aside is generally used for indirectly related content and footer is used at the end for information about author, links, contact details etc.
  2. I also find there is no heading in your page, each page should generally contain a h1 for the title, so I think the name of the person can be a h1 instead of p for the title.
  3. The image of the the person is not a decorative image so it should include a alt attribute to describe it for assistive technologies or when it fails to load there must be something to describe the image.

  Hope it helps

  1
• 

  kudos2Shef•210

  @kudos2ShefSubmitted almost 2 years ago

  

  HTML CSS FLEXBOX

  #accessibility

  3

  10
  

  Rohit Deshpande•870

  @rohitd99Posted almost 2 years ago

  Hi kudos2Shef

  Congrats on completing the challenge.

  I noticed that to center the card you've used properties like position : relative etc on your card, well to center something you don't need these but simply use a flex or grid on main

  
  main {
  min-height: 100vh;
  display: flex;
  flex-direction: column;
  justify-content: space-around;
  align-items: center;
  }
  

  add these to your solution and remove the position : relative and top : 120px from your card. Also I see you've used headings in the wrong way. The card title must he a h1 instead of an h3. Each page must have a single h1 heading for the title. Headings must be in order from h1 through h6. Same for the footer instead of h6 , I think semantically a p element should suffice as I don't think that is a heading.

  Hope it helps

  Marked as helpful

  0
• 

  Utkarsh Raj Mishra•30

  @UtkarshrajmishraSubmitted almost 2 years ago

  

  Responsive Product Preview Card using Flexbox

  1

  20
  

  Rohit Deshpande•870

  @rohitd99Posted almost 2 years ago

  Hi Utkarsh

  Congrats on completing the challenge.

  Enjoyed going through it, well structured, and responsive. But I have a few suggestions that will help you improve it.

  1. You've used div's for structuring but I'd suggest using main and section as they are semantic elements.
  2. For representing the deleted text use del instead of p.
  3. The spelling of Perfume is mistaken as Prefume.
  4. Also instead of having two images and changing them through media queries, I'd like to suggest using picture element with two different source's. Here's the MDN doc for the same picture.

  Hope it helps

  1
• 

  David•40

  @David23-DevSubmitted almost 2 years ago

  

  Responsive landing page using CSS flex

  2

  10
  

  Rohit Deshpande•870

  @rohitd99Posted almost 2 years ago

  Hi David

  Congrats on completing your first challenge 🥳🎉.

  Also no need to apologise for your english. Now coming to the challenge I have some suggestions for you.

  I see you've implemented the solution nicely for the mobile and desktop sizes but for tablet size they don't cover the entire width.

  @media screen and (max-width: 600px)
  {
  body{
  justify-content : start;
  }
  .contenedor {
  box-shadow: none;
  flex-direction: column;
  height: auto;
  /* width: auto; */
  width: 100%;
  justify-content: start;
  /* align-items: center; */
  }
  #tusResultados {
  border-top-left-radius: 0;
  border-top-right-radius: 0;
  width: auto;
  }
  #summarry {
  width: auto;
  }
  }
  

  I believe these properties should make it better on tablet sizes.

  Other than that I also see you've used headings in a wrong way, h1 heading must only be used once per page. All the headings must be in order that is h1 then h2 and so on. Here's a video for correct usage of headings.

  Overall you've done well for your first solution, used semantic elements. Keep going and you'll learn things in no time.

  1
• 

  Jan van Ierssel•20

  @JanvampiersselSubmitted almost 2 years ago

  

  NFT Card Design

  4

  20
  

  Rohit Deshpande•870

  @rohitd99Posted almost 2 years ago

  Hi Jan van Ierssel

  Congrats on completing the challenge.

  I just wanted to point out a few things, which can improve the solution.

  1. I noticed you've used h3 for the title, but an h1 heading would be more suited. For each page a single h1 heading is always expected and all the headings must be in order from h1 through h6. h1 generally goes for the title, h2 for subtitle and so on.
  2. Also on you've put height: 100vh on body , main but min-height : 100vh should be used if you want them to stretch as per content. Although on this challenge it wouldn't make much difference.

  Hope it helps

  Marked as helpful

  2
• 

  abd nour•160

  @alghaylanSubmitted almost 2 years ago

  

  qr-code-component

  1

  30
  

  Rohit Deshpande•870

  @rohitd99Posted almost 2 years ago

  Hi abd nour

  Just a small suggestion, I saw that you've used a h3 heading for the title , instead use an h1 heading. Each page must have a single h1 and all the other headings must be in order that is from h1 through h6. They carry meaning and <h1> defines the most important heading, and <h6> defines the least important heading. Thus h1 should be the most suited for the title.

  Hope it helped

  Marked as helpful

  0