Responsive order summary card

Community feedback

• 

  Shahin NJ•1,190

  @SJ-NosratPosted over 3 years ago

  Hi, Great solution! I have noticed the following:

  1. Why is this element: <a href="./index.html" class="btn btn-primary">Proceed to Payment</a> not a <button type="button" href="./index.html" class="btn btn-primary">Proceed to Payment</button>? Since you've used: <button> element for your Cancel Order button.

  2. I hope you're not using <strong class="body-text card-plan-type">Annual Plan</strong> the strong element for styling purposes? If you're using it to markup so that the browser draws the attention of screenreaders, then it is fine! Just wanted to give a headsup if it's being used for styling (that's what CSS is there for).

  3. Try and wrap the below code in <figure> tag instead of the <div> tag:

   <div class="card-plan-circle">
      <img src="./assets/icon-music.svg" alt="Music icon" />
   </div>
  

  Great work! Keep on coding!

  Marked as helpful

  1

  ---

  

  Leon Michalak•545

  @NinjaInShadePosted over 3 years ago

  @shahin1987

  1. My thinking is Cancel Order will run some function for example, it isn't neccessarily just to go to another page or part of a page. Whereas proceed to payment should take you to another page, so that's my thinking.

  2. Nope, not just for styling purposes, I used CSS for boldness :D Purely semantic :)

  3. Isn't figure meant for when there is also text with the image, so you add a <figcaption>?

  Thank you for the reply I appreciate it!

  1

  

  Shahin NJ•1,190

  @SJ-NosratPosted over 3 years ago

  @NinjaInShade

  1. Aha! I see, then you can still use the <button> tag with the following attribute: formtarget="_blank" so it will look like this:

   <button formtarget="_blank" type="button" href="./index.html" class="btn btn-primary">Proceed to Payment</button>
  

  The definition of the formtarget can be found here. Please be aware that there are other options too.

  2. That's great!

  3. Yes! you're right, however looking at the definiton of the Permitted content on MDN: A <figcaption> element, followed by flow content; or flow content followed by a <figcaption> element; or flow content.

  Where flow content that are permitted inside the <figure> tags are elements like: <img>, <p>, <h1>-<h6>, <button>, etc.

  If you have a semantic HTML element in mind and are not sure what is the permitted content allowed to be nested inside, always consult the documentation on MDN, when you've exhausted all options then opt for a <div> tag.

  I hope the above explains my thinking.

  1

  

  Leon Michalak•545

  @NinjaInShadePosted over 3 years ago

  @shahin1987 Yeah that explains it, I still think <a> is better in that case since anchor tags are used for navigating to different areas of a website though, but the other things you said were great!

  0
• 

  nmorajda•945

  @nmorajdaPosted over 3 years ago

  Difficult to navigate with the keyboard (tabulator) because for the .btn class you set outline: none without any additional highlighting.

  Marked as helpful

  1

  ---

  

  Leon Michalak•545

  @NinjaInShadePosted over 3 years ago

  @nmorajda Great spot! Going to get that sorted :)

  0

  

  Leon Michalak•545

  @NinjaInShadePosted over 3 years ago

  @nmorajda Should be fixed! focus states are one thing I've always missed, but now that I'm dedicated to better accessibility thanks for reminding me!

  0

  

  nmorajda•945

  @nmorajdaPosted over 3 years ago

  @NinjaInShade Adding a border for the last button may not be the best solution as it causes elements to move when the border appears.

  1

  

  Leon Michalak•545

  @NinjaInShadePosted over 3 years ago

  @nmorajda Yeah that can be fixed by setting a default border of 1px solid transparent so you cant see it but still takes up space, then change it to the dark colour. I'll fix it now :D

  0