Skip to content
OverviewSolutions
1
Comments
8
Ynnk
@HYannick

All comments

  • Ynnk•170
    @HYannickPosted over 4 years ago

    Love the style and the transitions on the cards :) the code structure is good as well. Maybe you could try context API for your theme switch, that could avoid the Redux use since here you only dispatch one action and render data through it

    1
  • Ynnk•170
    @HYannickPosted over 4 years ago

    Nice work on this!

    Maybe use more readable class name for &__h1-> &__title and &__p-> &__description

    On the JS side there could need some refacto on the error handling part.

    Maybe wrap on the html side the errors like so:

    <div data-error='firstname'  class='error'>
    <img id="img1" src="./images/icon-error.svg" alt="error-icon" />
    <span>First Name cannot be empty</span>
</div>

    and then on the js side use a validator function that will map the error depending of the form data provided, and add the error class with the right styles depending of the right data-attribute (I don't know if I am clear ahah)

    1
  • Magda Kokot•1,425
    @magdakokSubmitted over 4 years ago

    Hello people! :) I followed Matt's suggestion and coded mobile-first, what seems to be a bit more toilsome, but rewarding as well. For now, I know I have to work on consistency of BEM naming, there's form validation that needs improvement (I added an issue on GitHub repository).

    All suggestions are very, very welcome! :)

    Mobile first site using Sass and a bit of JavaScript

    5
    Ynnk•170
    @HYannickPosted over 4 years ago

    This is simply awesome! :D

    Some suggestion on the menu animation, it's usually better to use css transform as they are smoother and more performant than absolute positioning :).

    I found a little issue when you activate the menu on mobile and then expand your screen, the menu disappear but the overlay is still here.

    Also, it could be great to hide the menu when you click on the overlay :).

    On the JS side maybe you could toggle classes instead of apply style in plain javascript. something like nav__box--open and nav__box--closed, in order to make this more readable and BEM compliant.

    I love the flip animation on the about cards! This is really smooth and sick.

    2
  • Ynnk•170
    @HYannickPosted over 4 years ago

    Hi! Nice shot! I like the transition on the error/valid field.

    For your issue, I wondered why you didn't use the display: flex on the body?

    It seems here but not applied since left and right side have their own width and the right part a position: fixed.

    Maybe you could remove the widths and replace them with flex-basis or flex property since the purpose of flexbox is to have 'flexible' content inside. With this, the fixed position of the right side removed and a body with a min-height: 100vh the background will take the whole height.

    Hope that helps :)!

    1
  • Ynnk•170
    @HYannickPosted over 4 years ago

    Hi :D nice work !

    For the JS part you may not need this block :

    const card = document.querySelectorAll('.card');
card.forEach((card) => {
    card.addEventListener('mouseover', (event) => {
        card.classList.add("card-over")
        card.addEventListener('mouseout', (e) => {
            card.classList.remove("card-over");
        });
    });
});

    as you can do this in css like this instead of the "card-over" class:

    .card {
 // your beautiful css
 transition: transform 0.5s ease-out;
}
.card:hover {
    transform: scale(1.1);
    color: white;
    background-image: linear-gradient(135deg, hsl(236, 72%, 79%), hsl(237, 63%, 64%));
}

    For the toggle part I noticed a redundant process on the forEach. You could maybe refactor like this:

    const toggleDisplay = value => el => {
       el.style.display = value
   }
}

toggle.addEventListener("change", (e) => {
    if(e.target.checked) {
        annual.forEach(toggleDisplay("none"))
        monthly.forEach(toggleDisplay("block"))
    } else {
        annual.forEach(toggleDisplay("block"))
        monthly.forEach(toggleDisplay("none"))
    }
})

    Hope that helps!

    1
  • Alex•2,010
    @AlexKMarshallSubmitted over 4 years ago

    Hi, I approached the form validation here with vanilla js, and I think the error highlighting is handled ok. But it doesn't currently prevent the form from being submitted if you haven't filled it out correctly?

    Are there good strategies for preventing the form submission (not just disabling the button)?

    Thanks :)

    Vanilla JS sign-up form with CSS grid

    1
    Ynnk•170
    @HYannickPosted over 4 years ago

    Hi ! Great work :D

    Maybe add a event.preventDefault at the form submission ?

    Something like :

    const form = document.querySelector("form");

form.addEventListener('submit', (e) => {
 e.preventDefault();
 //Do your things
});
    2
  • Ynnk•170
    @HYannickPosted over 4 years ago

    Yeah I think the only way to get away with this is the absolute position :/ maybe a div with this background and a negative z-index and an overflow:hidden on it should do the trick

    0
  • Ynnk•170
    @HYannickPosted over 4 years ago

    This is a great result !

    Some suggestions would be to avoid overusing absolute position and margin positioning like this. In this case it does the tricks but it's hard to maintain especially if you want a more responsive display after that.

    For absolute position it's more conventional to use the top left right bottom css properties.

    Another potential issue would be the fixed size of the body. Usually it is meant to be flexible depending of the device.

    Another approach would be to use the flexbox technique. Here's some samples

    <div class="grid">
    <div class="grid-column">
        <div class="container container-left">
            <h2 class="box-title">Supervisor</h2>
            <p class="p-box">Monitors activity to identify project roadblocks</p>
            <img src="images/icon-supervisor.svg" alt="supervisor-img"> </img>
        </div>
    </div>
    <div class="grid-column">
        <div class="container container-top">
            <h2 class="box-title">Team Builder</h2>
            <p class="p-box">Scans our talent network to create the optimal team for your project</p>
            <img src="images/icon-team-builder.svg" alt="team-builder-img"> </img>
        </div>

        <div class="container container-bottom">
            <h2 class="box-title">Karma</h2>
            <p class="p-box">Regularly evaluates our talent to ensure quality</p>
            <img src="images/icon-karma.svg" alt="karma-img"> </img>
        </div>
    </div>
    <div class="grid-column">
        <div class="container container-right">
            <h2 class="box-title">Calculator</h2>
            <p class="p-box">Uses data from past projects to provide better delivery estimates</p>
            <img src="images/icon-calculator.svg" alt="calculator-img"> </img>
        </div>
    </div>
</div>

    styles.css

    body {
  color: #40514E;
  margin: 0;
  text-align: center;
  font-family: 'Poppins', sans-serif;
}

h2 {
  font-size: 15px;
}

p {
  color: grey;
  color: hsl(229, 6%, 66%);
  width: 33%;
  margin: auto;
}

img {
  position: absolute;
  margin: 40px 90px;
  width: 40px;
  height: 40px;
}

.box-title {
  color: black;
  color: hsl(234, 12%, 34%);
  text-align: left;
  margin-left: 30px;
  margin-top: 30px;
}

.p-box {
  color: grey;
  color: hsl(229, 6%, 66%);
  line-height: 2;
  text-align: left;
  width: 80%;
  font-size: 12px;
}

.h1-plain {
  color: grey;
  font-weight: lighter;
  margin-bottom: 10px;
}

.h1-bold {
  margin-top: 0;
}

.grid {
  margin-top: 50px;
  display: flex;
  justify-content: center;
}

.grid-column {
  display: flex;
  flex-direction: column;
  justify-content: center;
  margin: 0 15px;
}

.container {
  border-top: thick solid;
  background-color: white;
  box-shadow: 0 10px 10px rgba(0,0,0,0.15);
  height: 200px;
  width: 300px;
  margin-bottom: 30px;
}

.container-left {
  border-top-color: cyan;
  border-top-color: hsl(180, 62%, 55%);
}

.container-top {
  border-top-color: red;
  border-top-color: hsl(0, 78%, 62%);
}

.container-bottom {
  border-top-color: orange;
  border-top-color: hsl(34, 97%, 64%);
}

.container-right {
  border-top-color: blue;
  border-top-color: hsl(212, 86%, 64%);
}

.attribution {
  margin-top: 30px;
  font-size: 11px;
  text-align: center;
  color: purple;
  color: hsl(228, 45%, 44%);
}

    Have fun with the next challenges !

    1