Mobile-first with Flexbox and old-fashioned CSS

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Great work Emmilie Estabillo!

  1

  ---

  

  P

  Emmilie Estabillo•5,540

  @emestabilloPosted almost 3 years ago

  @PhoenixDev22 Thanks a lot! :-)

  0
• 

  Marcus Hugo•365

  @marcus-hugoPosted almost 3 years ago

  Hey Emmilie, I've been googling javascript for a basic carousel, and all the tutorials are very complicated, but, your solution is so concise! I couldn't understand how to implement the logic and I've been studying your solution and wondering how the slideIndex gets updated with each click?

  I think I understand. Is it because slideIndex is a variable that gets updated with each onclick="plusSlides(1) or onclick="plusSlides(-1)?

  1

  ---

  

  P

  Emmilie Estabillo•5,540

  @emestabilloPosted almost 3 years ago

  @marcus-hugo Hi Marcus, yes, I remember using this link as a reference https://www.w3schools.com/w3css/w3css_slideshow.asp. There's a brief explanation at the end that might help :-)

  1

  

  Marcus Hugo•365

  @marcus-hugoPosted almost 3 years ago

  @emestabillo Thanks a lot!

  1
• 

  Alex Kim•1,325

  @alex-kim-devPosted about 4 years ago

  What's everyone's take on the tablet layout for this project?

  I do basically the same as you did - leave the 1 column layout and give it max-width to prevent content (especially text) getting too wide + centering

  1

  ---

  

  P

  Emmilie Estabillo•5,540

  @emestabilloPosted about 4 years ago

  @Alex-K1m This is another project I want to go back and edit to incorporate all the comments here. Layout-wise, I think I will still stick to one column for tablet. Side by side at medium widths seems tight to me.

  1
• 

  Shashi Lo•1,345

  @shashiloPosted over 4 years ago

  Great job! At first glance it looks pretty darn close to the design, but I did see a few things that could use some extra attention.

  1. I'm curious why you set the html font-size to 38px and 32px for mobile. I usually set up my html at a default of 10px and adjust it from there.

  2. The content font colors look like they're not using the dark-blue CSS variable.

  3. You're missing the border-radius on the images.

  4. The body background leaks through the container. I believe it would be better if this background was on .slider.

  Other than that, I'd like to se a smoother transition instead of a display: hide/block;.

  1

  ---

  

  P

  Emmilie Estabillo•5,540

  @emestabilloPosted over 4 years ago

  Hi @shashilo,

  1. I was going by the style guide - the base font was 32px. I probably went backwards with this since I went mobile-first. For medium devices, I was still using the same stacked layout. So to save myself another x-lines of code trying to adjust each element, I just increased the root size.
  2. I missed this bit
  3. Another great catch! Will edit.
  4. I was thinking the same thing, but did it on the body anyway to match the desktop view. I'm probably going back to adjust the slider. Thank you for the pointers!!

  0
• 

  Roman Filenko•3,335

  @rfilenkoPosted over 4 years ago

  Hi EMMILIE, the issue is with .wrapper - height:100% on mobile casues problems, just disable it, otherwise looks great on mobile🙂

  Roman

  1

  ---

  

  P

  Emmilie Estabillo•5,540

  @emestabilloPosted over 4 years ago

  @rfilenko Awesome! Thank you for looking into it :-)

  0