Mobile first with bem and scss and javascript

Community feedback

• 

  eagermonument81•210

  @ixtkPosted over 3 years ago

  • header needs some black overlay on top because I can barely see white navigation links. It was in design too
  • the modal popup isn't centered and is very long, maybe it would make sense to add a scrollbar
  • button font-size is very small in modal
  • I would add cursor: pointer to the modal close button and radio checkbox section

  Also, check the report

  When it comes to folder structure, this from pro membership challenge files has become my new template

  - assets
    - mobile
    - tablet
    - desktop
    - shared (for icons that appear on all layouts, maybe logo.svg)
  - css
    - style.css
  - js
    - main.js
  .gitignore
  index.html
  

  Marked as helpful

  1

  ---

  

  Heritier Akilimali•310

  @heritioPosted over 3 years ago

  @ixtk Thanks for the feedback, seems like a rushed over those aspects, it look into it, and thanks for the file structure showcase

  0
• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted over 3 years ago

  Hello there, Heritier Akilimali! 👋

  Good job on this challenge! 👏 Your solution seems to work pretty well! 😀

  The only super tiny suggestion that I have is to perhaps add cursor: pointer to some of the interactive elements around the site (such as the pledge options and the close button inside the pledge popup). 🙂

  It also might be worth taking a look at your solution report and trying to clear up some of the errors that are there in order to improve the accessibility and semantics of your solution. 😉

  Keep coding (and happy coding, too)! 😁

  Marked as helpful

  0

  ---

  

  Heritier Akilimali•310

  @heritioPosted over 3 years ago

  @ApplePieGiraffe Thanks for the suggestion, il implement it on the next attempt, i ddint realize i hadd rushed over those details. thanks for your time

  1

  

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted over 3 years ago

  @heritio

  No problem! Glad to help! 😀

  0