QR Code Component

Community feedback

• Account deleted

  Hello there! 👋

  Congratulations on finishing your challenge! 🎉

  I have some feedback on this solution:

  • Always Use Semantic HTML instead of div like <main> <header> <footer> , etc for more info

  i hope this is helpful and goodluck!

  Marked as helpful

  2

  ---

  

  Dorian Arriaga•20

  @dorianarriPosted over 2 years ago

  @Old1337 Thank you! I'll keep this in mind from now on. 👍

  1
• 

  Dev Rathore•2,600

  @GitHub-dev12345Posted over 2 years ago

  If you want to reduce your accessibility, Change this code. 😊

  <div class="container"> to <main> ( Used main tag for main card design ) <div class="attribution"> to <footer> ( Used footer tag for footer design) I hope you find this helpful & Nice Work Keep it up 👍👍

  Marked as helpful

  1

  ---

  

  Dorian Arriaga•20

  @dorianarriPosted over 2 years ago

  @GitHub-dev12345 Thanks! I'll start using semantic tags more. I appreciate the input. 😄

  0
• 

  Svetlana Jokic•90

  @HolllyyyyPosted over 2 years ago

  Great job @dorianarri :3 You can add box-shadow if u would like it.

  1
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Greeting @dorianarri ,

  I have few suggestions regarding your solution:

  • To tackle the accessibility issues, you can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech).

  
  .sr-only {
  	border: 0 !important;
  	clip: rect(1px, 1px, 1px, 1px) !important;
  	-webkit-clip-path: inset(50%) !important;
  		clip-path: inset(50%) !important; 
  	height: 1px !important;
  	margin: -1px !important;
  	overflow: hidden !important;
  	padding: 0 !important;
  	position: absolute !important;
  	width: 1px !important;
  	white-space: nowrap !important;            
  }
  

  • You should use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

  • ` height: 490px;It's rarely ever a good practice to set heights on elements . Let the content inside the card element dictate the height of it.

  Overall , your solution is good.Hopefully this feedback helps.

  1

  ---

  

  Dorian Arriaga•20

  @dorianarriPosted over 2 years ago

  @PhoenixDev22 Interesting, I'll have to look into 'sr-only' and try to understand 'em' more. Thank you.

  0