Mobile_First_Profile_card_component

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  Hi

  I'm afraid you need to rewrite the html on this. All changes should be easy to do though

  • You need to Indent your code consistently. Your code editor can even do this automatically for you on save. This makes the code easier to read, is an established standard and prevents bugs from silly things like skipped closing tags which start to happen a lot on bigger pages
  • its extremely important to use appropriate meaningful html elements for the content. Only headings should be heading tags, and these must go in order, starting from h1 and never skipping levels
  • the stats should be 3 bullet points in a list, not headings and paragraphs. It makes no sense to have headings with numbers in like 300k. Those numbers can be wrapped in spans or strong tags to make them display block and acheive the styling
  • read up about landmarks. Your accessibility errors on this are because the component should be inside a main element and attribution should be inside a footer element

  Other general styling feedback is the shadow looks too dark compared to the design, and the card is touching my screen edges on mobile. There should always be a little space around it like the design

  Marked as helpful

  2

  ---

  

  scarydeveloper•130

  @snake321Posted over 2 years ago

  @grace-snow thanks for your feedback I will rewrite the code and will solve these issues and then I will send you the link then pls give me your feedback thank you so much for telling me my mistakes.....

  0

  

  scarydeveloper•130

  @snake321Posted over 2 years ago

  @grace-snow pls give me feedback now because I think I have solved all issues which you told me

  0

  

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  @snake321 I'm afraid this still isn't right

  • alt text is incorrect alt="victor_dp". Alt should be human readable text, it's not code for a computer. In this case it should be the person's name
  • you still have h2s with no content under them. That makes no sense at all. As I said previously, the stats need to be 3 bullets in a list. Turn off styling for the page and the structure should be understandable and formatted as if it was any other document
  • remove all those brs. None of them should be there. They are meaningful and will be readout as "break" to screenreaders which you rarely want to happen. Again as I said before you can wrap the stats numbers in span/strong and set to display block of you want them on their own line

  Marked as helpful

  1

  

  scarydeveloper•130

  @snake321Posted over 2 years ago

  @grace-snow Thank you for your feedback but I didn't understand this (Brs) would pls tell what do you mean of this

  0

  

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  @snake321 every time you have added <br/> that should not be there. It is extremely rare you need to use these, almost never.

  Marked as helpful

  1

  

  scarydeveloper•130

  @snake321Posted over 2 years ago

  @grace-snow Thank you so much for your such amazing feedback I think I have solve all issues which you told me so would you plz check my solution give me valuable feedback if I deserve ..... @grace-snow your feedback are always helpful so never stop to help needy people
  keep coding keep learning and keep helping

  0
• 

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  Much better now, well done!

  Marked as helpful

  1

  ---

  

  scarydeveloper•130

  @snake321Posted over 2 years ago

  @grace-snow Thank you so much

  0
• 

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  I am seeing lots more problems and not sure if they are new or were there before - css problems now.

  Why have you done font-size: 55%;?! That is never a good idea! You are introducing a huge accessibility problem if you forget to scale even one element up to correct font size again. There is no reason to do this and no benefits

  Why is there a width on the card in rem as well as a max width? If you want to include a width that should be 100%. Otherwise your max width has no effect

  Why is there an auto margin on the card? You are already centering the card on the page with flexbox so no need for this. The only need for margin would be a small fixed amount - or a small fixed amount of padding on an outer wrapper to prevent card hitting screen edges

  Why is body overflow hidden?

  These things are making content hidden for me on mobile because they overflow the viewport and scroll has been removed, also card is touching screen edges with no space around it.

  Marked as helpful

  1

  ---

  

  scarydeveloper•130

  @snake321Posted over 2 years ago

  @grace-snow the reason I am using font size 55% because when going on 1440px then I change my font size to 60% and then my all content increase automatically... I learnt this from responsive course Is it good or not but I helps a lot so pls tell me should I continue usage of font size like this otherwise solving another issues which you just told me now

  0

  

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  @snake321 I would never ever recommend scaling font size down at the root/html.

  You can override most accessibility issues bg scaling the font size up again immediately on the body (eg body {font-size: 1.82 rem;} )

  But not every element inherits font size, so you have to tread extremely carefully and make sure font size is scaled back up on those.

  There are waaaay better ways to deal with different screen sizes imo

  Marked as helpful

  1

  

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  @snake321 I would never ever recommend scaling font size down at the root/html.

  You can override most accessibility issues bg scaling the font size up again immediately on the body (eg body {font-size: 1.82 rem;} )

  But not every element inherits font size, so you have to tread extremely carefully and make sure font size is scaled back up on those.

  There are waaaay better ways to deal with different screen sizes imo

  Marked as helpful

  1

  

  scarydeveloper•130

  @snake321Posted over 2 years ago

  @grace-snow Hi snow thank you so much for your help I think I have solved all issues which you told me plz check my code now and then pls give honest feedback as you give thank you so much @grace-snow

  0