Develop #2639
Develop #2639
Conversation
There was a problem hiding this comment.
paddings on logo and menu
compare bikes section should be horizontal on wider screen
images should be smaller in details
contact us form should be side be side with phone, email and address
page shouldn't reload when submitting the form
form should have validation - no empty forms allowed
menu should work
phone, emial and address should be interactive (I should be able to call / send email after click)
|
As you can see i pulled request view hours ago, but then i made a new commit. Now i cant do pull request form vesrion 'almost final version, maybe some smallers things to do'. This one is right one |
There was a problem hiding this comment.
please checkout figma design, I think the desktop view looks little different, than the one you submitted
I shouldn't be able to send empty form, and when trying to do that, I should get an error message
I can't see bike pictures in compare section 🤔
phone, email and address in contact section should be clickable, and trigger actions
|
As you can see, I've corrected most of the errors, however, in the 'contacts us' section, the link redirecting to the email doesn't seem to be working. The address and phone number are more or less okay. I've searched the internet for ways to redirect it and found only <a href="mailto:..>. Is that a good way to do this ? |
There was a problem hiding this comment.
Your email redirection is working, but there are some other minor issues:
- checklist: Page shouldn't be reloaded on form submit (https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault)
- checklist: disable page scrolling under the menu
- phone number should be centered
src/index.html
Outdated
| <h4 class="address__label address__label--first"> | ||
| <a href="tel:+1 234 5555-55-55">Phone</a> | ||
| </h4> | ||
| <p class="address__content" id="number">+1 234 5555-55-55</p> | ||
| <h4 class="address__label"> | ||
| <a href="mailto:[email protected]">Email</a> | ||
| </h4> | ||
| <p class="address__content">[email protected]</p> | ||
| <h4 class="address__label"> | ||
| <a href="https://maps.app.goo.gl/uXQTCdJCemi7rnYj6">Adress</a> | ||
| </h4> | ||
| <p class="address__content">400 first ave.<br>suite 700<br>Minneapolis, MN 55401</p> |
There was a problem hiding this comment.
phone number, mail or address should work as links not their labels ;)
src/index.html
Outdated
| <li class="links__item"><a href="#details" class="links__link">Details</a></li> | ||
| <li class="links__item"><a href="#contact-us" class="links__link">Contacts</a></li> | ||
| </ul> | ||
| <div class="links__item links__item--number">+1 234 5555-55-55 </div> |
| <button | ||
| class="button button--primary" | ||
| type="submit" | ||
| id="button-primary"> |
There was a problem hiding this comment.
format code correectly
add names to your inputs
src/index.html
Outdated
| <h4 class="address__label"> | ||
| Phone | ||
| </h4> | ||
| <p class="address__content" id="number"> |
There was a problem hiding this comment.
why do you need additional p for links? can't you just style <a> element?
it's a good practice to avoid html elements where they are not needed to prevent excessive-DOM size
|
I fixed problem with header background image in my last commit after I requested 'add solution' :) |
Project type: MyBike