Skip to content
OverviewSolutions
14
Comments
14

Agustin

@agusc01Argetina340 points

Je m'appelle Agustín

Latest solutions

View more solutions

Latest comments

  • Agustin•340
    @agusc01

    Hi Iva,

    Here's a tip for your third question

    • how to force to close the accordion item/line if another one is opened (so only one accordion item is opened)?

    The answer like your coding (with "for" and "this." selector)

    const accordion = document.getElementsByClassName('accordionItem');

const closeAllItems = () => {
  for (i = 0; i < accordion.length; i++) {
    accordion[i].classList.remove('active');
  }
};

for (i = 0; i < accordion.length; i++) {
  accordion[i].addEventListener('click', function () {
    let wasItOpen = this.classList.contains('active');
    closeAllItems();
    if (wasItOpen === false) {
      this.classList.add('active');
    }
  });
}

    And another way to writing code (which in my opinion is better, using foreach, so I didn't use "this." selector)

    
const accordions = document.querySelectorAll('.accordionItem');

const closeAllItems = () => {
  accordions.forEach((accordion) => {
    accordion.classList.remove('active');
  });
};

accordions.forEach((accordion) => {
  accordion.addEventListener('click', function () {
    let wasItOpen = accordion.classList.contains('active');
    closeAllItems();
    if (wasItOpen === false) {
      accordion.classList.add('active');
    }
  });
});

    Keep coding and have fun !

    Marked as helpful
  • Agustin•340
    @agusc01

    Hi the below sentence code is wrong...

    emailError.textContent = 'Email should be at least ${email.minLength} characters';

    you have to use `` instead of ' '.

    Interpolation only works with ``.

    Marked as helpful
  • Agustin•340
    @agusc01

    Hi !

    All your images are empty in alt="" attribute. It's very important for 2 reasons .

    1. search engines need it to understand what is in the photo
    2. when a person is blind, he/she can't see your image but he/she can hear the content (with a particular configuration to blind people's devices)

    Then, you wrap the image into a div.img-box, the search engines migth not understand the context of your web site I recomend you change this

    <div class="icons">
  <div class="img-box">
    <img src="./images/icon-document.svg" alt="">
  </div>
  <div class="img-box">
    <img src="./images/icon-folder.svg" alt="">
  </div>
  <div class="img-box">
    <img src="./images/icon-upload.svg" alt="">
  </div>
</div>

    for this

    <div class="icons">
  <button class="icon__button"><img class="icon__image" src="./images/icon-document.svg" alt="document icon"></button>
  <button class="icon__button"><img class="icon__image" src="./images/icon-folder.svg" alt="folder icon"></button>
  <button class="icon__button"><img class="icon__image" src="./images/icon-upload.svg" alt="icon upload"></button>
</div>

    then change your css with this

    .icon__button {
  width: 45px;
  height: 45px;
  border-radius: 8px;
  display: grid;
  background-color: var(--Very_Dark_Blue);
  border: none;
}
.icon__image {
  margin: auto;
}
.icon__button:hover {
  cursor: pointer;
}

    If you do this , you will see that nothing changes BUT now you will have more specific content for search engines and blind people , also you have a nice effect in your buttons with the property "cursor:pointer"

    Anyway your challenges looks great . Keep coding

    Marked as helpful
  • Agustin•340
    @agusc01

    Hi Asha.

    In your script.js change this

    const arrows = document.querySelectorAll('.arrow'); 

arrows.forEach( arrow => {
arrow.addEventListener('click', () =>{
    let parent = arrow.parentElement; 
    parent.parentElement.classList.toggle('active'); 
})
})

    for this

    const firstLines = document.querySelectorAll('.first-line');
firstLines.forEach((arrow) => {
  arrow.addEventListener('click', () => {
    let parent = arrow.parentElement;
    parent.classList.toggle('active');
  });
});

    Then remove function removeClasses (you didn't use it)

    In your css file add this

    .first-line:hover {
  cursor: pointer;
}

    For you <div class="cointainer">....</div> try to add a relative width like width:28rem for example

    Try to use a border-bottom in your <div class="question-box"> instead creating a <div class="divider"></div> . Think abou this... to create a question box you need a <div> you never will forget this element because everything is inside. But you maybe create a div.question-box and forgot div.divider, then looks bad ... If you have the line divider inside the div.question-box with a border-bottom you never forget it . I know, you may think that looking at the page on a server you will notice the difference that a divider is missing, but when you spend hours and hours programming, you will not see it even if you have it in front of your eyes

    Sorry for my poor english it isn't my first language.

    Keep coding. ! Practices makes perfect !

    Marked as helpful
  • Agustin•340
    @agusc01

    Hi Asha.

    All images have empty alt="", iIt is important for search engines to understand what is in the photo and also for blind people

    Wrap each image icon into a anchor like <a href="#"> <img src="images/icon-facebook.svg" alt="icon facebook"></a>

    This will look better .share:hover { cursor: pointer; }

    And look what happens (on mobile version) writting this @media (max-width: 850px) { //... .info-and-share:not(.active) { margin-bottom: -0.8rem; } }

    You can add this

    @media (max-width: 850px) { img { //... border-radius: 10px 10px 0 0; } }

    In my opinion you should use relative units instead absolutes units

    Keep coding ! Great job !

    Marked as helpful
  • Agustin•340
    @agusc01

    a person uses media queries to change properties if they don't change the properties media queries are not used. I mention it because there is unnecessary code like

    .body{ background-image\: url(./bg-desktop.svg); background-repeat: no-repeat; }

    then @media screen and (min-width: 768px) { body { background-image: url(./bg-desktop.svg); .... background-repeat: no-repeat; } ..... }

    And you know what...again....

    @media screen and (min-width: 375px) and (max-width: 767px) { body { .... background-image: url(./bg-mobile.svg); background-repeat: no-repeat; } ..... }

    the properties and values ​​did not change why would you write the same thing again??? and that is not the only case you repeat this pattern more than once. Today it might be nothing, but in the future with a big project you will go crazy

    It is not good that you use a class for a <div> and an <a> I do not see why they would share the class ".icons"

    I recommend that you put .icons-box{ text-decoration:none} to remove the underline

    Try to use classes as much as possible and avoid ID's

    Writing javascript in the HTML is absolutely no good, neither is putting CSS in your HTML (like inline styling). Now it's nothing but in bigger project , it's necessary split it

    First you should solve the previously mentioned problem of the "icons" class being in one <div> and in 3 <a>. I would put something like this

    <div class="icon"> <a class="icon__link" href="www.facebook.com" target="_blank"> <i class="icon__italic fab fa-facebook-f fa-lg" ></i> </a> <a class="icon__link" href="www.twitter.com" target="_blank"> <i class="icon__italic fab fa-twitter fa-lg" ></i> </a> <a class="icon__link" href="www.instagram.com" target="_blank"> <i class="icon__italic fab fa-instagram fa-lg" ></i> </a> </div>

    javascript-file

    const iconLinks = document.querySelector(".icon__link") iconLinks.forEach(link => { link.addEventListener("mouseover",()=>{ link.classList.add("mouse-over-colour") }) link.addEventListener("mouseleave",()=>{ link.classList.remove("mouse-over-colour") }) });

    css-file .mouse-over-coulour{ color='#E882E8'; }

    the default color (white) it'll on the class ".icon__italic" for example

    And you have to create folders to separate contents . You really need it

    Keep coding . ! No one was born knowing

View more comments