Skip to content
Submitted almost 3 years ago

Responsive Order Summary Component using clamp

Robert McGovern•1,075
@tarasis
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Order summary component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

Would love feedback on avoiding the VW magic numbers for clamp. And also about my use of BEM. This is my first real attempt at using it and I'm stumped whether the price-plan area should be considered its own block, likewise the button area.

So then price-plan, with price-plan__icon, price-plan__plan-type, price-plan__plan-price, price-plan__change-button? Likewise button-area, button-area__payment-button, button-area__cancel-button? Not keen on it to be sure.

Outside of that, any other improvements I could make?

(There are two images in the design folder showing the diffs between design image and final, darker is mine)

Community feedback

  • Grace•31,330
    @grace-snowPosted almost 3 years ago

    Hi

    This is hitting all screen edges on my mobile in portrait and the image is cut off on mobile landscape

    In the html, you must not use div/span alone for text. What are meaningful elements you could use in that price box?

    Curious here - what do you think would happen if someone clicked “change” if this was a real project?

    The BEM looks fine on this, but don’t be afraid to start/use a new block if you need to. Like that pricing grey box could easily be its own component used elsewhere on a site.

    Similarly, on things like buttons/links you might find there are classes established for those styles that are reusable, and a BEM class would only be used if they needed changing/modifying. Eg a project might have a .btn class you’d use, then you’d also give it a bem class scoped to this component to control things like margin top/bottom on that button

    Marked as helpful
    1
    Grace•31,330
    @grace-snowPosted almost 3 years ago

    Looking at css I can see the source of the mobile landscape issue straight away. Never use this

    width: 100vw;
height: 100vh;

    Height instead of min-height is stopping content from growing.

    And 100vw is always a bad idea because it doesn’t account for scrollbars or device notches.

    Other thing I noticed in the css if you’re changing more in the media query than expected. Eg why would hover style not be on links all the time? There’s no reason to limit it like that.

    Make sure you always include obvious focus-visible styles on all interactive elements as well - very important for keyboard users

    Good luck

    Marked as helpful
    1
    Robert McGovern•1,075
    @tarasisPosted almost 3 years ago

    @grace-snow first thank you for the incredibly helpful notes. I've made some changes based on them. (Not BEM yet)

    1. Media Query Good point on the hover/focus states. The reason I put them in the media query was that the design only showed the desktop version as having those states. I simply took it too literally without thinking about it.

    I've tidied that up and actually swapped a couple of other bits in to variables which are just changed in the media query.

    1. the 100vw/vh

    Interesting and thanks for the heads-up, I hadn't seen that effect in Safari (Desktop & iOS), Firefox or ResponsivelyApp. I started with using 100% for both but it broke vertical centering, so I swapped to vh and didn't think about it. I think the issue was something else but I didn't spend the time investigating then.

    1. Don't use Span/Div for text ...

    I will get that drilled into me yet. I mentally balk at the idea of p because I think paragraph, and a few words or even a single word isn't a paragraph. Just need to repeat until I get to the point of not using div/span

    1. What would happen on clicking change?

    I don't know and hadn't considered it. A dialog appearing, originating from the change button and using z-index, to allow changing between a few options. I don't know if it would be a local change, or a remote routed change.

    1. BEM

    Thanks for the useful notes on BEM.

    0
    Grace•31,330
    @grace-snowPosted almost 3 years ago

    @tarasis great, that all makes sense.

    So if you think change would open a modal (or toggle the price to monthly maybe), it would be a button, not anchor tag 😉

    Marked as helpful
    1
    Robert McGovern•1,075
    @tarasisPosted almost 3 years ago

    @grace-snow 🤦‍♂️ <<< me right now

    1
  • Naveen Gumaste•10,480
    @NaveenGumastePosted almost 3 years ago

    Hay ! Good Job you made it look nearly perfect to the preview

    -> Distance between music logo and "Annual Plan" is to much try to reduce it

    -> Increase the font size of the "change" around 0.35rem more

    Keep up the good work!

    Marked as helpful
    1
    Robert McGovern•1,075
    @tarasisPosted almost 3 years ago

    @Crazimonk thank's.

    Totally missed that the Change button was different sized for the Desktop compared to the Mobile. Fixed, also reduced the box sizes around the price plan and "Proceed to Payment" button.

    The other bit I'll take a stab at fixing a bit later, once I work out the best method to just nudge the "Annual Plan" over and up a bit.

    0

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord