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

Обработка ошибок GraphQL #120

Merged
merged 12 commits into from
Nov 25, 2020
Merged

Conversation

niyazm524
Copy link
Collaborator

Closes #4
Closes #31
localhost_3000_login

  • Компоненты для home page переиспользованы
  • Добавлены новые ошибки (блин)

@niyazm524 niyazm524 self-assigned this Nov 25, 2020
@niyazm524
Copy link
Collaborator Author

#88 - на секунду показывается страница UserInfo

@github-actions
Copy link

github-actions bot commented Nov 25, 2020

Visit the preview URL for this PR (updated for commit 23998d0):

(expires Tue, 15 Dec 2020 19:04:00 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@azinit
Copy link
Member

azinit commented Nov 25, 2020

Хмм))

image

UPD: Чекни этот компонент, который для HomePage и для ошибок юзается

Чет с размерами тут

image

@azinit
Copy link
Member

azinit commented Nov 25, 2020

Пока некритично, но до показа бы исправить, что на какое-то время мелькает сама страница

Т.е. в идеале должна быть сначала проверка на доступ к ресурсу (для 404), и только если ок - уже отображаем сам page

(может имеет смысл какой-нить флаг accessLoading добавить на верхнем уровне, и только как она станет false - отображать чайлдов гарантированно)

В рамках PR - пофиг пока, но ишью бы выписать

#88 - на секунду показывается страница UserInfo

Да, да - поправить бы - выпиши issue себе

Copy link
Member

@azinit azinit left a comment

Choose a reason for hiding this comment

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

Все круто, но го еще добьем моменты некоторые, чтоб точно до ума довести

src/app/error-handling/error-catcher.tsx Show resolved Hide resolved
src/app/error-handling/error-catcher.tsx Show resolved Hide resolved
src/app/header/index.tsx Show resolved Hide resolved
src/app/header/index.tsx Outdated Show resolved Hide resolved
src/app/header/index.tsx Outdated Show resolved Hide resolved
src/app/utils.scss Outdated Show resolved Hide resolved
src/app/index.tsx Outdated Show resolved Hide resolved
src/features/home-hero/index.tsx Show resolved Hide resolved
src/pages/home/index.scss Show resolved Hide resolved
src/pages/home/index.tsx Outdated Show resolved Hide resolved
@azinit azinit added this to In progress in v.0.1.X. - Improved via automation Nov 25, 2020
@azinit azinit added this to the Спринт 2020_6 milestone Nov 25, 2020
Copy link
Member

@azinit azinit left a comment

Choose a reason for hiding this comment

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

Вливай. Остальное позже пофиксим

Молодец 👍

Реализация - пушка

import withRouter from "./with-router";

// Потом какой-нибудь `compose` метод заинсталлим откуда-нить и покрасивше будет
export const withHocs = (component: () => JSX.Element) => withRouter(withApollo(component));
Copy link
Member

Choose a reason for hiding this comment

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

Некритично, но в react-app-env уже задеклейрен глобально тип для компонентов

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

там с пропсами

Copy link
Member

Choose a reason for hiding this comment

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

Так можно же props?: P выставить)

@@ -34,7 +34,7 @@ const client = new ApolloClient({
/**
* Обертка для подключения и работы с API
*/
const withApollo = (component: () => React.ReactNode) => () => (
const withApollo = (component: () => JSX.Element) => () => (
Copy link
Member

Choose a reason for hiding this comment

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

Некритично, но в react-app-env уже задеклейрен глобально тип для компонентов

image

import withApollo from "./with-apollo";
import withRouter from "./with-router";

// Потом какой-нибудь `compose` метод заинсталлим откуда-нить и покрасивше будет
Copy link
Member

Choose a reason for hiding this comment

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

😄

@@ -3,10 +3,12 @@ import React, { Suspense } from "react";
import { BrowserRouter, Route } from "react-router-dom";
import { QueryParamProvider } from "use-query-params";

export const setupRouter = (component: () => JSX.Element) => () => (
const withRouter = (component: () => JSX.Element) => () => (
Copy link
Member

Choose a reason for hiding this comment

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

Потом бы добавил коммент, аля

/**
 * Инициализация роутера с провайдером для работы с get-параметрами
 */

import Header from "./header";
import "./index.scss";
import { withHocs } from "./hocs";
Copy link
Member

Choose a reason for hiding this comment

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

Круто, но стили в самом низу располагают обычно)

imports/order увы не удается пока для этого настроить

import Header from "./header";
import "./index.scss";
import { withHocs } from "./hocs";

const ErrorPage = lazy(() => import("pages/error"));
Copy link
Member

Choose a reason for hiding this comment

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

Чет смущает, что ErrorPage тут так хардкодится... Но ладно, потом посмотрим что с этим можно сделать. Потом в след коммитах пометь как-нибудь

// !!! FIXME: manage access

q: currentTarget.value,
type: query.type,
s: query.s,
o: query.o,
Copy link
Member

Choose a reason for hiding this comment

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

Потом бы на такое заменил

const q = currentTarget.value;
history.push(`/search?${qs.stringify({ ...query, q })}`);

@niyazm524 niyazm524 merged commit cc9e19c into dev Nov 25, 2020
v.0.1.X. - Improved automation moved this from In progress to Done Nov 25, 2020
@niyazm524 niyazm524 deleted the feature/error-handling branch November 25, 2020 19:54
niyazm524 added a commit that referenced this pull request Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment