Skip to content

solution Ihor Sobko (partial react Uber Eats)#27

Open
igaryok wants to merge 34 commits into
mate-academy:masterfrom
igaryok:fs_on_jull19_ihor_s
Open

solution Ihor Sobko (partial react Uber Eats)#27
igaryok wants to merge 34 commits into
mate-academy:masterfrom
igaryok:fs_on_jull19_ihor_s

Conversation

@igaryok

@igaryok igaryok commented Oct 22, 2019

Copy link
Copy Markdown

-DEMO

@irenhh irenhh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. add transition on hover effect
  2. customize outline or remove it
  3. if you don't have sign-in page, don't redirect to empty page with error
  4. when you go to the restaurant page you need to go to the top of it, not the place you were on the prev page
  5. pay attention to the button, text is not centered + inputs don't fit the page width
    image
  6. on mobile screens there is no indentations between restaurants' cards

Comment thread src/store/actions.js Outdated
Comment thread src/components/CardItem/index.js Outdated
Comment thread src/components/Error/Error.js Outdated
Comment thread src/components/Header/Header.js Outdated
Comment thread src/components/Input/Input.js Outdated
Comment thread src/components/RestaurantCard/RestaurantCard.js Outdated
Comment thread src/components/RestaurantPage/RestaurantPage.js Outdated
Comment thread src/components/RestaurantPage/index.js Outdated
Comment thread src/components/RestaurantsListPage/RestaurantsListPage.js
Comment thread src/components/Section/Section.js Outdated
igaryok and others added 5 commits October 22, 2019 16:27
Co-Authored-By: Iryna Holovko <33497431+irenhh@users.noreply.github.com>
Co-Authored-By: Iryna Holovko <33497431+irenhh@users.noreply.github.com>

@irenhh irenhh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the first two comments were missed

if you see a comment to some part of your code, that means you have to consider it not only to that part but also to the rest of your code

Comment thread src/components/Select/Select.scss Outdated
Comment thread src/components/Section/index.js Outdated
Comment thread src/components/RestaurantsListPage/RestaurantsListPage.js
igaryok and others added 2 commits October 24, 2019 09:57
Co-Authored-By: Iryna Holovko <33497431+irenhh@users.noreply.github.com>

@irenhh irenhh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you still have commented code

Comment thread src/components/CardItem/index.js Outdated
Comment thread src/components/RestaurantsListPage/RestaurantsListPage.js
@danheim

danheim commented Oct 30, 2019

Copy link
Copy Markdown

Если обновить вкладку, то ресторан не загружается. Добавь плавную прокрутку при нажатии на раздел меню. (scroll-behavior).

По коду пятой задачи замечаний нет.

@igaryok

igaryok commented Oct 30, 2019

Copy link
Copy Markdown
Author

Если обновить вкладку, то ресторан не загружается. Добавь плавную прокрутку при нажатии на раздел меню. (scroll-behavior).

По коду пятой задачи замечаний нет.

пофиксил.
страница при перезарузке не обновлялась - проблема с BrowserRouter на gitHub - перешел на HashRouter, но появилась проблема с хэшякорями в меню на странице ресторана - меню переделал на js
smooth скрол добавил

@danheim

danheim commented Oct 31, 2019

Copy link
Copy Markdown

https://www.loom.com/share/30293ec5d3c44e7098aa8db3004b7a08
Это не является частью пятой задачи, поэтому ожидаю исправления в шестой 👍

@igaryok

igaryok commented Nov 1, 2019

Copy link
Copy Markdown
Author

https://www.loom.com/share/30293ec5d3c44e7098aa8db3004b7a08
Это не является частью пятой задачи, поэтому ожидаю исправления в шестой

refctored

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.

3 participants