Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

myPR #6

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

myPR #6

wants to merge 7 commits into from

Conversation

koretskyi
Copy link

No description provided.

Copy link

@irenhh irenhh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://prnt.sc/ote5s6 no extra indentations for header, it must have the same width as main block and footer

https://prnt.sc/ote6x6 the top of the image is cut off + 'some address' is not a good placeholder

https://prnt.sc/ote7lp add left indentations for user text

https://prnt.sc/ote7zo fix the styles according to the mockup

https://prnt.sc/otebqj user text must be black + width of the input has to be the same as the width of the whole block

https://prnt.sc/oteceb remove outline from here or make the same outline for every input

https://prnt.sc/oteifc left indentation must be the same for these blocks, check the mockup

https://prnt.sc/otemep make sure these block have the same indentations

https://prnt.sc/oteoqn make sure these images look the same as in the mockup

https://prnt.sc/oteq3z fix this (from 641px to 1024px) and the same with footer

https://prnt.sc/oterbc you don't have this block

https://prnt.sc/oterl7 why is it centered?

Check alt-attribute everywhere, it must contain some content.

<header>
<div class="header box-content">
<a href="#" name="back_to_top" class="logo">
uber <span>eats</span>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use image from mockup for logo

<div class="asap">
<input type="button" name="asap__button" value="ASAP" class="filead asap__button button">
<p class="asap__text">to</p>
<div class="asap__adres filead">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asap__adres -> asap__address

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is filead?


</div>
</form>
<div class="inits_button">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename inits-button to something more recognizable

<div class="inits_button">
<a href="#" class="sign-in filead button">Sign in</a>
<a href="#" class="register filead button">register</a>
<a href="#" class="basket"><img src="./img/icon/basket.svg" alt=""></a>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use <button> tag for buttons

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nested tags place on the new line!

</header>
<main>
<div class="box-content">
<div class="search">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add margin to this block, no need to add padding to each child element separately

<p class="menue__item__time"> 25-35 Min </p>
</div>
<div class="menue__item">
<img src="images/menue__items__img/95b745f6ce6a45903c3ed254afd72e46-w550-272.png" alt="">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't leave empty alt

<div class="menue__item">
<img src="images/menue__items__img/95b745f6ce6a45903c3ed254afd72e46-w550-272.png" alt="">
<p class="menue__item__restaurant">WOKWEI</p>
<ul class="menue__item__tag one__tag">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not a list

</div>
<div class="footer__info">
<ul>
<li class="select__lang">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is wrong BEM

<ul>
<li class="select__lang">
<img class="globe" src="images/icon/Shape1.svg" alt="">
<select class="" name="">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. make <select> dropdown width the same as in the mockup
  2. https://prnt.sc/otel0l this arrow has to work when you click on it

<a href="#">Privacy</a>
<a href="#">Terms</a>
</div>
<a href="#back_to_top" class="button__up">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

place this on the bottom on the page and don't stick it, because it covers other content

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants