Skip to content
OverviewSolutions
9
Comments
47

Eugenia

@JaneMoroz950 points

👋 Hi! I am a front-end developer and a student of freeCodeCamp🎓.

I’m currently learning...

Currently I'm learning three.js 🤓

Latest solutions

View more solutions

Latest comments

  • Eugenia•950
    @JaneMorozPosted over 1 year ago

    Hey Zet! Wow, your solution is awesome ❤️

    Some things I've noticed:

    • I think I would just add a bit of top padding to the header or nav
    • And often this kinda of logos are supposed to be links (which lead to Home page).

    About the circle number icon:

    • I think the way I would do it:
    <div class="section-number">01</div>

    // The circle with the number inside
.section-number {
  position: relative;
  height: 40px;
  width: 40px;
  display: flex;
  align-items:center;
  justify-content: center;
  border: 1px solid grey;
  border-radius: 50%;
}

// The line
.section-number::before {
  position: absolute;
  display: block;
  bottom: 100%;
  left: 50%;
  transform: translateX(-50%);
  content: " ";
  height: 70px;
  width: 1px;
  background-color: grey;
}

    This is not styled exactly, just trying to give some general idea.

    Keep it up! And good luck 🍀

    Marked as helpful
    1
  • Eugenia•950
    @JaneMorozPosted over 1 year ago

    Hey! Your solution is great 😊

    Frameworks can make your life easier, especially when the project is big. I used to code big projects with Vanilla JS and in the end I kinda had to create some sort of framework myself, without it it's hard to follow DRY principle.

    Some people don't like frameworks for various of reasons. One reason is that they're changing often and you need go though docs again or some dependencies stops working together etc.

    Marked as helpful
    0
  • Eugenia•950
    @JaneMorozPosted over 1 year ago

    Hey! Very nice solution 🎉

    • I guess you got some 'landmark something warnings'. I get them when I forget to wrap the main content of the page into the main tag. So I suggest you to use main tag instead of div with class="main"
    • Instead of h3 I suggest to use h1 and then style it.
    • Maybe it's better to wrap Jules Wyvern into the a instead of p since it seems to be a link and then style it because link tag usually has default underlying etc.

    Keep it up! And good luck 🍀

    1
  • Eugenia•950
    @JaneMorozPosted over 1 year ago

    Hello! Cool solution to the challenge 👍

    I've noticed that the submit button works the first time but not the second. So it seems like the problem is that after we click 'Dismiss message' and go back to the initial form, event listeners are not attached. I think this is because the form we're going back to is not the initial one but just newly created (with function pageSwapPrevious()).

    • So maybe one solution is to make sure that newsletter is not removed when success shows up. To do so you need to give success element position: absolute and switch between display:none and display: something else instead of changing document.body.innerHTML .
    • Another solution is to move subscribeButton.addEventListener logic into onSubmit() function for example, you will also need to add inputElement and invalidMessage:
    function onSubmit() {
  const inputElement = document.querySelector('.js-input');
  const invalidMessage = document.querySelector('.invalid-email');

  const regx = /^([a-zA-Z0-9\._]+)@([a-zA-Z0-9])+.([a-z]+)(.[a-z]+)?$/
  
  if (inputElement.value.match(regx)) {
    pageSwapNext();
    return;
  } else {
    invalidMessage.innerHTML = 'Valid email required'
  } 
}

    And then just add it to the button's onclick: <button onclick="onSubmit()" class="subscribe js-subscribe-button">Subscribe to monthly newsletter</button>

    Keep it up! And good luck 🍀

    1
  • Eugenia•950
    @JaneMorozPosted over 1 year ago

    Hey! Your solution is great 🤩

    The only things I've noticed are some accessibility issues:

    • You can wrap the main content (which is QR Code in this case) into the main tag or replace existing div tag with main.
    • Also usually the page should have h1 tag. You have h2, I guess you decided to use it because it has desirable font-size but you can style h1.

    Overall everything is great and I like that it is responsive ☺️

    Keep it up! And good luck 🍀

    Marked as helpful
    1
  • Eugenia•950
    @JaneMorozPosted over 1 year ago

    Hello Richard! I love your solution to this challenge ❤️

    I think I found a solution to the replies width bug. If you remove the default replies and create new reply you'll see that they are not filling the whole horizontal space.

    The reason replies are kinda taking all horizontal space in the second comment (with default replies) is because default replies kinda stretch Replies_repliesContainer so the new replies (children) just fill the container's width.

    So in order to solve this problem you need to make Replies_repliesContainer take the whole width of it's parent. For example you can add width: 100% to Replies_repliesContainer and CommentAndReply_container. Or you can add flex:1 to Replies_repliesContainer, this way Replies_repliesContainer will just take the rest of flex-container space.

    Keep it up! And good luck 🍀

    P.S. You can also use gap property to add the space between the vertical line and the reply.

    0
View more comments