Zaytsev header #9

zytsev · 2024-08-24T12:46:40Z

Create layout: header, footer, main page.

netlify · 2024-08-24T12:46:53Z

✅ Deploy Preview for rest-graphql-client ready!

Name	Link
🔨 Latest commit	`5aab976`
🔍 Latest deploy log	https://app.netlify.com/sites/rest-graphql-client/deploys/66ccb0495b4d96000887b727
😎 Deploy Preview	https://deploy-preview-9--rest-graphql-client.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

bananazigzags · 2024-08-24T13:31:15Z

.eslintrc.json

@@ -17,6 +17,7 @@
    "react/static-property-placement": 0,
    "react/require-default-props": 0,
    "react-hooks/exhaustive-deps": 0,
+    "linebreak-style": 0,


ты уверен в этой настройке? я вообще не знала, что такая есть (в настройках я видела обычно только lf, если настраивали), но тут в комментах к ответам, которые ее предлагают, говорят, что она плоха
https://stackoverflow.com/questions/39114446/how-can-i-write-a-eslint-rule-for-linebreak-style-changing-depending-on-windo

непонятная настройка. Специально ее не ставил. Возможно, появилась, когда автоматически разрешались зависимости.

bananazigzags · 2024-08-24T13:34:20Z

src/app/globals.css

-  html {
-    color-scheme: dark;
-  }
+button {


всем кнопкам в проекте будет задан margin? обычно привычнее, что элементы, особенно такие базовые, не задают сами себе маржины

не настаиваю. нужно было раздвинуть кнопки. сделаю контейнер

bananazigzags · 2024-08-24T13:37:50Z

src/app/layout.test.tsx

+    render(<RootLayout children={undefined} />);
+
+    expect(screen.getByRole('img', { name: 'RSSchool' })).toHaveProperty('alt', 'RSSchool');
+    expect(screen.getByText('2024')).toBeDefined();


тут наверное лучше использовать toBeInTheDocument, или по какой причине выбран toBeDefined?

верно. сперва хотел поставить библиотеку .../jest-dom. там есть эти методы. Но библиотека нормально не встала, показались ошибки зависимостей. В документации NEXT JS предлагается toBeDefined...

желательно поисследовать, как можно это решить. Сейчас те пакеты, что есть, все равно плохие (ошибки при установке). А где предлагается в документации? Если не получится нагуглить решения, можно пока оставить так, потом если получится, все пофиксить.
Вот какой-то ишью похожий, пока я его весь не просматривала
testing-library/jest-dom#515

https://github.com/vercel/next.js/blob/canary/examples/with-vitest/__tests__/Home.test.tsx

вот их пример. Я согласен что надо настроить jest dom

bananazigzags · 2024-08-24T13:52:26Z

src/app/layout.test.tsx

+import { render, screen } from '@testing-library/react';
+import RootLayout from './layout';
+
+describe('Layout component', () => {


и тут выглядит так, что компонент layout ответственен только за то, чтобы отрисовать футер. Но у него же есть header и children. Тут можно добавить пару простых тестов на это. Header можно замокать вообще, просто проверить что был вызван (с нужными пропсами если они есть). И проверить, что отрисовал детей, вложив ему какой-то текст и проверить что этот текст есть.

bananazigzags · 2024-08-24T14:07:21Z

src/app/page.test.tsx

+  test('Page renders', () => {
+    render(<Page />);
+
+    expect(screen.getByRole('heading', { level: 1 })).toBeDefined();


у некста такие тесты что ли в базовом приложении? странно, для меня это очень странно такое видеть в тестах DOM. to be defined, это больше про переменные. Я бы использовала toBeInTheDocument, если нет где-то инфы, что по какой-то причине это предпочтительно.
Ну вот тут говорят, что от tobedefined может быть много false positives.
https://medium.com/@jameswinston/quick-tips-tobedefined-or-not-tobedefined-that-is-the-question-tips-for-false-positives-in-jest-6f07025ea705

выше написал

bananazigzags · 2024-08-24T14:09:05Z

src/components/header/header.module.css

+    align-items: center;
+    justify-content: space-evenly;
+    width: 100%;
+    background-color: #eaeae4;


вот эту переменную хардкодом я уже видела, можно было бы завести css variable под это, и везде использовать ее (чтобы если захотелось изменить background цвет приложения везде, нужно было бы менять только одно место)

bananazigzags · 2024-08-24T14:10:29Z

src/components/header/header.tsx

+import { Button } from 'antd';
+
+function Header() {
+  const [Y, setY] = React.useState(0);


Y не очень имя, в коде ниже сразу непонятно. Лучше назвать scrollY, setScrollY хотя бы

bananazigzags · 2024-08-24T14:16:20Z

src/components/header/header.tsx

+    const scrollValue = window.scrollY;
+    setY(scrollValue);
+  }
+  React.useEffect(() => {


тут не критично, но можно импортировать только то, что нужно (деструктурировать import {useState, useEffect} from 'react', и не нужно все время писать в коде React...
Если у вас у всех есть предпочтение писать так, то ну можно (но тогда нужно чтобы везде было консистентно), но особо не вижу смысла в лишних буквах

bananazigzags · 2024-08-24T14:18:10Z

src/components/header/header.tsx

+    return () => window.removeEventListener('scroll', onChangeY);
+  }, []);
+  return (
+    <header className={`${style.header} ${Y > 0 && 'issticky'}`}>


тут при Y 0 будет стиль style.header false. Ну, конечно он вряд ли что-то себе поймает, но можно сделать для такого варианта пустую строку просто. Или использовать библиотеку classnames и с ее помощью собирать

bananazigzags · 2024-08-24T14:21:03Z

src/components/switchLang/switchLang.tsx

+}));
+
+function SwitchLang() {
+  //const language = useLang((state) => state.language);


лучше когда создаешь PR, просматривать его, чтобы не было таких мест (ниже тоже есть забытый закоментированный код)

bananazigzags · 2024-08-24T14:21:52Z

src/components/switchLang/switchLang.tsx

+  return (
+    <form className={style.form}>
+      <label className={style.label}>
+        <input type="radio" name="lang" defaultChecked={true} value="en" onChange={(e) => setLanguage(e.target.value)}></input>


у тебя тут хэндлер переключения языка три раза копипастится. Можно его вынести в handleLangChange и везде использовать

либо тут вообще можно было бы сделать массив языков, и далее просто по ним мапиться, т.к. у тебя здесь у этих трех инпутов отличается только value и label и что en default checked, остальное копипаста

bananazigzags · 2024-08-24T14:26:04Z

src/components/switchLang/switchLang.tsx

+  setLanguage: (language: State['language']) => void;
+};
+
+export const useLang = create<State & Action>((set) => ({


тут надо посмотреть, куда складывать этот стор. Если все будут его импортировать из компонента switchlang, это будет очень странно. Лучше если он будет где-то лежать отдельно, и уже оттуда этот компонент будет импортировать useLang и далее те, кто будет пользоваться этим языком (intl provider или что-то такое)

Если Zustand - правильнее делать один большой стор по типу Redux?

bananazigzags · 2024-08-24T14:59:16Z

.husky/_/pre-commit

а что делает это? я хаски можно почти не пользовалась. Тут нужно убирать совсем npx lint-staged? Я нашла какую-то статейку, где была эта строка и далее команда, которую выполняем (т.е. тут npx lint-staged).
попробовала сейчас, тогда просто ничего не происходит. Если вернуть - тогда выполняется что-то, если пытаюсь делать коммит. Это похоже ошибка

не менял эти настройки.

bananazigzags · 2024-08-26T18:04:50Z

src/app/globals.css

 }
+
+.button_welcome_wrapper {


это же не глобальный стиль, этот стиль к page относится. Его можно в page.module.css сложить и можно сделать buttonWrapper просто. Ну или wrapper будет в том css module, который будет, когда этот блок кнопок будет сложен в shared (чтобы его можно было вставить и на странице и в хедере, вроде так же обсуждали)

zytsev added 5 commits August 21, 2024 12:48

init: create branch

7906e1f

fix: fix errors Eslint

32c2ab6

feat: add layout components

ac79800

feat: add header scroll animation

d1d94c5

feat: add tests

ca66872

zytsev requested review from petru6e4ka and bananazigzags August 24, 2024 12:55

bananazigzags reviewed Aug 24, 2024

View reviewed changes

fix: fix after mentors checking

5aab976

bananazigzags reviewed Aug 26, 2024

View reviewed changes

petru6e4ka closed this Aug 29, 2024

petru6e4ka deleted the zaytsev_header branch August 29, 2024 11:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zaytsev header #9

Zaytsev header #9

zytsev commented Aug 24, 2024

netlify bot commented Aug 24, 2024 •

edited

Loading

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 25, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

bananazigzags Aug 24, 2024 •

edited

Loading

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 24, 2024

zytsev Aug 25, 2024

bananazigzags Aug 26, 2024

		}

		.button_welcome_wrapper {

Zaytsev header #9

Zaytsev header #9

Conversation

zytsev commented Aug 24, 2024

netlify bot commented Aug 24, 2024 • edited Loading

✅ Deploy Preview for rest-graphql-client ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bananazigzags Aug 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Aug 24, 2024 •

edited

Loading

bananazigzags Aug 24, 2024 •

edited

Loading