Single Price Component

Community feedback

• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted almost 4 years ago

  Awesome work on this challenge, Szymon! You've done a really good job and your solution looks great. It sounds like you've been very intentional in what you wanted to implement and work on, which is great!

  What do you mean about the voiceover? What is it saying vs what you expect it to say? Typically it would say something along the lines of "List item, 1 of 5". I would always default to using the correct semantics based on the content. So a ul would be correct for the Why Us list of features/benefits.

  I've got a couple of pointers about your code after taking a look at your project:

  • The span inside the .card-1__text paragraph isn't necessary. I'd just let the width of the parent dictate when the text wraps as opposed to forcing it to display as a block element.
  • Beware of nesting your selector unnecessarily in Sass. Nesting is an amazing feature of Sass, but it can easily be misused. For example, in the generated CSS file, you've got a selector of .wrapper-component .main-component .card-1__heading-primary which could just be .card-1__heading-primary.

  I hope these tips help. Let me know if you have any questions. Keep up the great work!

  1

  ---

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @mattstuddert

  I have read these two articles: first, second. I was afraid that list-style: set to none can remove the semantic meaning of ul/li.

  I started sass a few weeks ago. To be honest I was thinking about this unnecessary nesting because at the end css file has got lots of classes as you mentioned it. So please tell me what is a good practice? How I should rewrite my sass without deep nesting?

  Thank you so much for your feedback :D

  1

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted almost 4 years ago

  @SzymonRojek thanks for the links! I didn't know that, so I've learned something new today! 🙂

  It seems like it's only a Webkit thing, but if you did need the semantics intact then you could use role="list" and just ignore the validator. It won't be taking into consideration that nuance, so you'd be perfectly fine adding it.

  With regards to the nesting, I'd just separate out a lot of the selectors to the own rules to reduce nesting. For selectors with the same beginning, you can use Sass to nest and group similar selectors without increasing the specificity of the selectors. For example:

  .card-1 {
      /* .card-1 styles */
  
      &__heading-primary {
          /* .card-1__heading-primary styles */
      }
  
      &__subtitle {
          /* .card-1__subtitle styles */
      }
  }
  

  This allows you to group selectors while also keeping the generated CSS to a single class.

  I hope that helps!

  1

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @mattstuddert

  I am very glad that you've learned something new. Yes, it is about Safari browser - so in this case we have to ignore a validator. That's good to know because I thought that a validator it's like a last "oracle" ;)

  I will have a look my sass again and possible rewrite it. Lots of things to learn Matt.

  Thanks so much :D

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted almost 4 years ago

  @SzymonRojek you’re welcome! Validators lack context for these edge cases. They can also be a bit behind best practices as the web evolves. They’re very helpful, but there are certainly times when they can be ignored 🙂

  1

  

  T

  Grace•31,250

  @grace-snowPosted almost 4 years ago

  @mattstuddert @SzymonRojek there is an alternative fix for the Voiceover list semantics issue where you hide the list style on the list items instead of the list itself, and use a zero-width-non-joiner symbol in unicode as the content in a pseudo element... 🙄

  I've used both approaches but honestly think I prefer this one. Just adding the roles to force the semantics seems a much simpler solution than faffing with the css that much!

  But some people prefer the css solution because the issue is caused by css so they think it should be solved in css, not html (and then you don't get the validator issues!) 😂

  2

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  Thank you so much Grace - you are the STAR! :D I will implement role="list" to ul, and role="listitem" and probably I will stop care about the HTML validator. Now I have to learn the BEM convention because it is quite tricky. I think I can leave this project just like that but if I can do something better then I prefer to do it 😂

  Thanks Matt. I have removed this span tag from the paragraph (it was too much), so instead of it I put br with display: none on mobiles so the p tag can be one with two sentences and after 420px I set it to display:block => is great! I don't like to use br but here IMO it is good idea to use it. Also I was reading about Sass nesting selectors - thanks again for this hint.

  Thanks again. Both of you guys - are amazing :D

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted almost 4 years ago

  @grace-snow that's good to know! I hadn't seen that approach before, so cheers for mentioning it!

  0