@riencoertjens
Posted
overall the code looks clean and readable, naming of classes is clear. Some feedback points I had
- think about the box model: consider each element as a rectangular box, padding is used to create space inside the box, margin is used to create space between sibling elements. in this case I would apply a padding to the white box that contains all the information. then use margins on the elements inside this box to create space between the siblings.
- re-usability: consider this task part of a big application, are there elements you would want to reuse (for example the buttons): try to create class names that you can reuse and combine (for example
btn
that create's the shape and general style of the button, then you can havebtn-primary
where you apply the blue background color,btn-secondary
would apply a transparent background that way you can doclass="btn btn-primary"
to create a button and reuse it somewhere else - overall structure: I don't have access to the design file, but generally designs like this have a "rythm", for example all elements within the white box will be spaced the same (about 24px in this one, you could create a
ordersummary-child
class to take care of that spacing) - units: I see you're using
px
,rem
andem
, it's best to stick to eitherrem
orpx
(prefereblyrem
) be careful withem
as it can have unexpected results - class vs id: you're using
class
most of the time, but then for theordersummary
child elements you switched toid
, best to stick to class for styling (either one works, but as a rule of thumb)
Marked as helpful
@CherieRose
Posted
@riencoertjens Hey Rien, thank you so much for your detailed feedback! Based on your comment, I altered a few things and added some padding to the container class which meant I could remove it from a lot of the elements contained within.