Order Summary Component

Community feedback

• 

  T

  Grace•31,090

  @grace-snowPosted 7 months ago

  I'm afraid this is overflowing my screen horizontally quite a lot because you've used an explicit width instead of max-width. But first, think more carefully about the html

  • all content should be contained within landmarks. This should be within a main. Every page must have one.
  • read the article in the resources channel on discord about how and when to write alt text on images. The alt text on both images needs to change.
  • It may be fine to use a h2 for the heading if you envisage this being below other content on a page, but I think that is unlikely. "order summary" sounds like a single page in an ordering process, so I would expect it to be a page title (h1).
  • should annual plan be a heading or a paragraph? Your choice, but it shouldn't be a strong only. That is an emphasis tag that goes inside other elements.
  • not necessarily a problem but I want to ask you to explain something: why have you chosen an anchor for "change", a button for "proceed" and a button for "cancel"? What would you expect to happen when users click each of these?
  • it's better for performance to link fonts in the html head instead of css imports.

  Now onto css:

  • get into the habit of including a full modern css reset at the start of the styles in every project.
  • mobile styles should be the default. Note the design and starter files include a different background image for mobile vs desktop and slightly different padding.
  • the care must have a max width in rem not a width. This allows it to shrink narrower when needed (eg on a smaller screen)
  • the card should have a little margin on all sides so it can't hit screen edges (or a wrapping element / body can have a little padding for the same effect)
  • does container need to be a flex column? I see no benefit when you're not using the gap property. If you did use gap you would remove vertical margins on children.
  • you asked about the difference between padding and margin. Hopefully that helps.
  • it's fine for line height to be in rem, but its unusual. Usually it's unitless like 1.5, or sometimes in % like 150%.
  • use gap and flex properties (like flex grow) instead of large margin on plan container child elements.
  • I think the proceed button should at least have a little inline padding. often padding on elements like this will be in px if you want it to be static or em if you want it to scale with the button text size.
  • this needs a media query as mentioned above. When you add one make sure it's min-width and that it's defined in rem or em not px. https://fedmentor.dev/posts/responsive-meaning/

  0