Officelite coming soon site

Community feedback

• 

  P

  Dave•5,295

  @dwhensonPosted almost 3 years ago

  Hey @stephjoseph - lovely job here. The page looks great - I like it!! 🥳

  Here's some feedback I hope you might find useful:

  1. You might want to think about stopping the page from spreading too wide a very large screens. This is an issue on the bottom of the first page and the second. There are many ways to do this but I set a grid on the body element, with three columns, as using a class selector as follows:

  .center-content {
  	display: grid;
  	grid-template-columns: minmax(1rem, 1fr) minmax(375px, 1440px)minmax(1rem, 1fr);
  }
  
  .center-content > *  {
  	grid-column: 2;
  }
  

  Other people use container classes to do the same thing. This article has a good run down of alternative approaches https://css-tricks.com/the-inside-problem/ You will note I am actually using the approach the author doesn't like!

  2. For the JavaScript, you've a lot of repetition on both pages. I would suggest looking into es modules here. That way you could separate out, for example the count down function, and then import the code into both files from one place. This would make your code easier to maintain, and should enable you to keep the counters in time between pages.

  If you've no experience using a bundler, I have found Parcel.js a good place to start as it's fairly easy to get up and running. This is a good thing to learn as you'll want to use a more modular approach as the challenges get more complicated.

  I've never used Tailwing, but almost 2000 lines of CSS seems a lot, and Parcel might be able to remove some uneeded code too (never tried this though!)

  3. Some other small thoughts on your JS:

  • I think if you used padStart you could avoid the ternary statements on your counter numbers? This can work for all the counters and should be fairly straight forward to add.
  • I would suggest creating an error class in CSS, then using JS to add/remove that class rather than writing out the CSS to add for each validation - just easier to maintain is all.
  • I would consider abstracting all the checking and validation code into a re-usable function - anytime you are repeating so much code it's worth considering if this is possible.

  But, don't let these small points take away your achievement here! This is a tough one and you did it nicely!! Great job 👍

  Hope this helps a little.

  Cheers 👋

  Dave

  Marked as helpful

  2

  ---

  

  stephmunez•435

  @stephmunezPosted almost 3 years ago

  Hi @dwhenson, really appreciate the constructive feedback.

  Yes, I was having a hard time figuring it out and left it as is. Didn't know there are many approaches for it until you shared, many thanks.

  Great suggestion on importing the code from one place to avoid repetition and for it to be easier to maintain, it did not cross my mind. Also will look up Parcel for sure, head of it but never tried haha

  For Tailwind yes it's a lot of CSS but compared to the whole file it's actually smaller because it's added just in time or "on the go" instead of having the whole CSS setup being downloaded/added thus only adding the classes that are used, I guess the best thing one can do is to minify it or maybe use it with Parcel, just like you said, it might do the trick as they are compatible!

  You're right padStart might be the easier option for it, I just read the docs. As well as creating an error class instead of explicitly adding/removing every class. Will take those points into consideration.

  Thank you so much Dave, it actually means and helps a lot! 👍🏽

  1

  

  P

  Dave•5,295

  @dwhensonPosted almost 3 years ago

  @stephjoseph happy to have helped!! I'll have to check out Tailwind, seems people really like it and I have no idea how it works! Cheers.

  0

  

  stephmunez•435

  @stephmunezPosted almost 3 years ago

  @dwhenson you should! Saves you a lot of time and you'll never want to write a line of CSS after, I can tell you that for free haha

  0
• 

  LiBee•370

  @Li-BeePosted almost 3 years ago

  Really good 👍👏. I see you set the image of the circles at the top as an ‘<img>’ and not a ‘<background-image>‘ may I ask why for my own understanding. Something I am struggling with at the moment.

  Curious did you take a course for learning html css if so can you let me know which one?

  0

  ---

  

  stephmunez•435

  @stephmunezPosted almost 3 years ago

  Hi @Li-Bee, thanks for the compliment. I set it as <img> and not as a background-image because the size of the image of the circles is large and with its position it needs to overflow the parent <div> whereas if I set it as background-image it will be limited within the <div> and cannot overflow for which it is the background.

  Actually, I did not take any course for learning html and css. I only used the book Learning Web Design - A Beginner’s Guide to HTML, CSS, JavaScript, and Web Graphics by Jennifer Niederst Robbins. It's very beginner friendly, each concept are explained thoroughly, and useful exercises are available within each chapter. I strongly recommend it 🙂

  Hope this helps, happy coding!

  1

  

  LiBee•370

  @Li-BeePosted almost 3 years ago

  @stephjoseph Oh... because if you used background-image it would stay in the container on the right-hand side?

  Good advice on the book will look at it.

  0

  

  stephmunez•435

  @stephmunezPosted almost 3 years ago

  @Li-Bee yes exactly 👍🏽

  0