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

BOX-185: Доприкрутил API вместо mock-заглушек [part 2] [final] #56

Conversation

azinit
Copy link
Contributor

@azinit azinit commented Aug 12, 2021

DISCLAIMER

Изменений не так много, в основном опять же - из-за кодогенерации)

Этот файлик можно проигнорить при ревью
image

CHANGELOG

Можно по коммитам отследить :)

  • Доприкрутил оставшиеся эндпоинты
  • Поправил модель данных
  • Немного порефачил типы
  • Minor refinements
    • Пофиксил тени
    • Добавил обработку defaultavatar
    • Поправил отображение скелетонов

@azinit azinit changed the title BOX-185: Доприкрутил API вместо mock-заглушек BOX-185: Доприкрутил API вместо mock-заглушек [part 2] [final] Aug 12, 2021
Copy link
Contributor Author

@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.

self-reviewed

Comment on lines +25 to +43
export interface Question {
readonly topic: string;
readonly author: CommentUser;
readonly when: string;
readonly text: React.ReactNode;
readonly resolved?: boolean;
readonly responses: {
authors: string[];
count: number;
lastReponseAt: string;
};
}
export interface Answer {
readonly author: CommentUser;
readonly title: string;
readonly when: string;
readonly why: 'liked' | false;
readonly text: React.ReactNode;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Позже прикрутим реальную АПИху для комментов

Comment on lines +8 to +15
// FIXME: remove component as redundant later

interface Props {
cards: Card[];
getHref?: (data: Card) => string | undefined;
getUserHref?: (data: Card) => string | undefined;
// FIXME: will be removed later
getUser: (data: Card) => User;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Меня оч корежит этот пропс-дриллинг и файлы **-list, поэтому позже как удалим - это автоматически устранится

Comment on lines +137 to +144
{/* FIXME: resolve better later */}
<Editor value={content as EditorValue} readOnly={true} />
</>
)}
{size === 'small' && (
<ItemEditorContainer>
{/* FIXME: resolve EditorValue BOX-185 */}
<Editor value={content as any} readOnly={true} />
{/* FIXME: resolve better later */}
<Editor value={content as EditorValue} readOnly={true} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Хз как лучше зарезолвить, ибо это генерится с typed-contracts

Comment on lines +71 to +76
// FIXME: move logic to entities level?
sample({
source: cardsSearchFx.doneData,
fn: ({ answer }) => answer.users as User[],
target: userModel.updateMap,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Для KV кэша

Comment on lines +23 to +46
const topCards = useStore(model.$topCards);
const latestCards = useStore(model.$latestCards);
const usersMap = useStore(userModel.$usersMap);

// FIXME: temp handlers
const handleUser = useCallback(
(card: Card) => {
const user = usersMap[card.authorId];
return user;
},
[usersMap],
);

const handleUserHref = useCallback(
(card: Card) => {
const user = usersMap[card.authorId];
return paths.user(user.username);
},
[usersMap],
);

const handleCardHref = useCallback((card: Card) => {
return paths.card(card.id);
}, []);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Самому оч не нравится, но когда задекомпозим нормально по слоям - и страница более тонкой станет

Comment on lines +18 to +20
// FIXME: move to entities/card level later? (as cache store?)
export const $topCards = createStore<Card[]>([]);
export const $latestCards = createStore<Card[]>([]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Пока показалось, что такое лучше всеж хранить на уровне страницы

Comment on lines +96 to +109
const CardResults = () => {
const usersMap = useStore(userModel.$usersMap);
const cards = useStore(searchModel.$cardList);
const isLoading = useStore(model.$isShowLoading);

return (
<CardList
cards={cards}
getHref={(card) => paths.card(card.id)}
loading={isLoading}
getUser={(card) => usersMap[card.authorId]}
/>
);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: По совету @sergeysova

Comment on lines 47 to 68
{Boolean(socials.length) && (
<SocialStaff>
<SocialStaffTitle>Social staff</SocialStaffTitle>
<SocialStaffList>
{socials.map((social) => (
<SocialStaffItem key={social.id}>
<SocialLink href={social.link}>
{avatar && <Avatar size="small" src={avatar} />}
<SocialStaffItemText>
@{social.username}
</SocialStaffItemText>
</SocialLink>
</SocialStaffItem>
))}
</SocialStaffList>
</SocialStaff>
)}
</UserSocial>
{avatar && (
<UserLogo>
<StAvatar size="large" src={avatar} />
</UserLogo>
)}
{/* FIXME: move to entities/user logic */}
<UserLogo>
<StAvatar size="large" src={avatar || imgLogo} />
</UserLogo>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Тож декомпозируем позж при рефакторинге

Comment on lines 31 to 37
// FIXME: Временный хак, чтобы запросить сессию с клиента для одной страницы
// (пока что нужно только на CardViewPage, но позже будет фетчится нормальным способом, когда подтянем авторизацию)
sample({
source: pageLoaded,
target: sessionModel._sessionLoadedClient,
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Если есть получше методы обойти это - пишите)

@@ -241,7 +241,6 @@ export const cardsSearchOk = typed.object({
roles: typed.array(typed.string).maybe,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Можно игнорить файл при ревью (codegen)

export const $currentUser = createStore<User | null>(null);

// FIXME: temp solution
export const $usersMap = createStore<Record<string, User>>({});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Потом мб уберем KV store, посмотрим

Comment on lines +1 to +3
import imgLogo from './logo.png';

export { imgLogo };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: Про проблемы реэкспорта помню, но и ассеты по проекту плодить не хочется "по месту использования")

@azinit azinit requested a review from dmi-ch August 12, 2021 18:23
@azinit azinit merged commit dd48cc7 into master Aug 13, 2021
@azinit azinit deleted the feature/box-185-прикрутить-реальную-api-на-смену-моковым-2 branch August 13, 2021 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants