Ibrahim Morad

Latest comments

• 

  Zahra Abellou•250

  @zahraabelluSubmitted 16 days ago

  

  News homepage

  #accessibility#sass/scss

  1

  30
  

  Ibrahim Morad•380

  @IbrahimMuradPosted 13 days ago

  Nice job.

  • Using a grid layout will make the components a bit nicer.
  • Also, body element should consist of a header, main, and footer. Using main inside section is not ideal. main should consist of sections, section should have a header h1-h6 and articles, article may have a header, and a paragraph p, and so on.

  Marked as helpful

  0
• 

  saccoViola•90

  @saccoViolaSubmitted 24 days ago

  

  Responsive contact form

  #accessibility#animation#lighthouse#van-js

  2

  20
  

  Ibrahim Morad•380

  @IbrahimMuradPosted 16 days ago

  Looks good. But you need some adjustments for better results.

  1. Add novalidation to the form since you are validating manually.
  2. Remove the action attribute; you are not making any requests.
  3. The event listener should be 'submit' on the form, not the 'click' on the button.
  4. aria-descripedby takes the id of the error element; you passed the id of the same element.
  5. Since you added required to the inputs, you just need to use inputElement.validity.valid to check if the value is valid (in this case, it will ensure that the value is provided and the email is a valid email).
  6. Give each label for the radio input a unique for value (for example, the value itself - general or support-), then add an id with the same value so that when you click on it, the value is selected.
  7. I noticed that you, sometimes, uses <br />, it works but you can wrap the elements that you want in the same line and use CSS to style it to stay in the same line, and also with CSS make sure the next element is in its own line.
  8. some other styles are missing, but I think its fine.

  Let me know if you need more clarifications.

  1
• 

  saccoViola•90

  @saccoViolaSubmitted about 1 month ago

  

  Faq questions

  #accessibility#lighthouse

  1

  20
  

  Ibrahim Morad•380

  @IbrahimMuradPosted 29 days ago

  You forgot to use the background color light-pink. Overall, nice work. Keep up.

  Marked as helpful

  0
• 

  Heba Ahmad Saleh•250

  @hebaahmedsalehSubmitted about 1 month ago

  What are you most proud of, and what would you do differently next time?

  ..

  What challenges did you encounter, and how did you overcome them?

  ..

  What specific areas of your project would you like help with?

  ..

  

  interactive-rating-component-main

  #accessibility

  1

  30
  

  Ibrahim Morad•380

  @IbrahimMuradPosted about 1 month ago

  Great job. Keep up.

  0
• 

  Michael Wilson•260

  @mikewil245Submitted about 2 months ago

  What challenges did you encounter, and how did you overcome them?

  Challenges i encountered was trying to figure out how to get the value from the custom tip input , and figured out that i had to add an event listener.

  

  Mobile First approach Tip Calculator | M/W

  2

  20
  

  Ibrahim Morad•380

  @IbrahimMuradPosted about 2 months ago

  I looked at you code again. It looks a lot better.

  Here are some other suggestions for improvements:

  1. You should remove class="tip-btn" from the custom tip input, this makes this input takes the styles of other buttons. Let it take the common style of input[type="number"], which you did, but the class ruins it when hovered.

  2. Add error messages to other inputs, same as number of customers, and for the style, do not be afraid of breaking everything, I found that all inputs of type number are the same, all labels are the same, and all error messages are the same, you will just add other error messages the same way you did with number of people.

  3. number of people has more than probable bad inputs, zero, negative, and decimal. So, I would recommend leave it empty in the HTML and fill it based on the input, "can't be zero", "can't be negative", or "must be a whole number".

  4. (I am not a js expert but in this project) I found that using Number to convert input value into number is much better, ParseFloat was not predictable for me. Numbers in js can be integers and floats, and the input with type number can not accept alphabets, so Number is more convenient.

  5. Use const instead of let. Any js styling checker will murder you if it sees your js script, they hate it.

  let numOfPeopleValue = parseFloat(numOfPeople.value) || 1; // Default to 1 if invalid
  
    if (!numOfPeopleValue) {
      errorMessage.classList.remove("hidden");
  
      tipTotal.textContent = "$0.00";
      total.textContent = "$0.00";
      return;
    }
  

  Regardless of this project, the condition in if statement will always be false, because parseFloat(numOfPeople.value) has two boolean options, true if it is a value or false if it is NaN, but you did add || 1, so if it is NaN, it will become 1, so !numOfPeopleValue will always be false.

  Marked as helpful

  0
• 

  Michael Wilson•260

  @mikewil245Submitted about 2 months ago

  What challenges did you encounter, and how did you overcome them?

  Challenges i encountered was trying to figure out how to get the value from the custom tip input , and figured out that i had to add an event listener.

  

  Mobile First approach Tip Calculator | M/W

  2

  20
  

  Ibrahim Morad•380

  @IbrahimMuradPosted about 2 months ago

  • You need to add more validations to the inputs, for example, number of people can not be negative or decimal values, bill can not be negative, and custom tip can not be negative and can not exceed 100.

  • Some styles are not applied, such as the chosen tip button should have a different color to indicate it has been chosen.

  Overall, great job. Keep up.

  Marked as helpful

  0