Skip to content
This repository has been archived by the owner on Sep 11, 2023. It is now read-only.

feat/landing-7 #34

Merged
merged 39 commits into from
Sep 10, 2023
Merged

feat/landing-7 #34

merged 39 commits into from
Sep 10, 2023

Conversation

oxiqod
Copy link
Contributor

@oxiqod oxiqod commented Aug 8, 2023

No description provided.

@oxiqod oxiqod self-assigned this Aug 8, 2023
@oxiqod oxiqod linked an issue Aug 8, 2023 that may be closed by this pull request
5 tasks
@oxiqod oxiqod removed a link to an issue Aug 8, 2023
5 tasks
@oxiqod oxiqod linked an issue Aug 8, 2023 that may be closed by this pull request
5 tasks
@oxiqod oxiqod requested a review from Nelfimov August 11, 2023 21:01
Copy link
Contributor

@Nelfimov Nelfimov left a comment

Choose a reason for hiding this comment

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

Много багов в дизайне:

  1. Заголовок индекса - на весь экран моего ноута (1440х900):

image

  1. Верхнее меню - его появление растягивает вьюпорт в ширину, так же не работает листание курсов:
Screenshot 2023-08-14 at 09 37 56
  1. Параллакс слишком быстро перекрывает низ блока:
Screenshot 2023-08-14 at 09 38 14 Screenshot 2023-08-14 at 09 38 30
  1. Нет картинок у курсов:
Screenshot 2023-08-14 at 09 38 20
  1. В мобильном виде верхняя кнопка "Курсы" с переносом
Screenshot 2023-08-14 at 09 40 39
  1. Поиск в библиотеке - при активности границы не подсвечиваются как в дизайне, кнопка с переносом слова:
Screenshot 2023-08-14 at 09 41 41

@oxiqod
Copy link
Contributor Author

oxiqod commented Aug 14, 2023

@Nelfimov
поправила, насчет карточек - их ж нет в fullHD, изображения в карточках по макету отображаются при вью от 2560px

@oxiqod oxiqod requested a review from Nelfimov August 14, 2023 12:45
@oxiqod
Copy link
Contributor Author

oxiqod commented Aug 17, 2023

@Nelfimov
по вышеуказанным комментам - правки внесла, раз уж все данные с сервера получаем, то что насчет странички Library? ее верстка почти полностью статична

@Nelfimov
Copy link
Contributor

Сделал данные для Library:

Hero:

query NewQuery {
  sections {
    nodes {
      sections {
        title
        navigationName
      }
      content
      id
    }
  }
}

Материалы:

query NewQuery {
  allTutorials {
    nodes {
      learningMaterials {
        description
        skills {
          ... on Skill {
            title
          }
        }
      }
      id
    }
  }
}

@oxiqod oxiqod requested a review from Nelfimov August 18, 2023 13:18
id: el.id,
}))

const reverseLinks = getLinks?.reverse()
Copy link
Contributor

Choose a reason for hiding this comment

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

Нехорошо привязываться к reverse, лучше элементы зашить четко по ID.

return (
<>
<Column display={sendForm ? 'none' : 'flex'}>
<Input
value={question}
onChange={setQuestion}
placeholder={intl.formatMessage({ id: 'questions.form.input.placeholder.message' })}
placeholder={form?.data?.allForms.nodes[6].forms.text}
Copy link
Contributor

Choose a reason for hiding this comment

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

Здесь и далее - ты зависишь от порядка ответа сервера. Лучше перейти на привязку к ID.

Copy link
Contributor Author

@oxiqod oxiqod Aug 29, 2023

Choose a reason for hiding this comment

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

@Nelfimov
не совсем понимаю как это реализовать, я получаю массив с объектами, внутри каждого хранится id:

"nodes": [
        {
          "id": "cG9zdDozMDA=",
          "forms": {
            "text": "Мы получили твой вопрос, вернёмся с ответом asap."
          }
        },
        {
          "id": "cG9zdDoyOTk=",
          "forms": {
            "text": "«Соглашение об обработке персональных данных»"
          }
        },
        {
          "id": "cG9zdDoyOTg=",
          "forms": {
            "text": "Нажимая на кнопку «Оставить заявку», вы принимаете"
          }
        },
        {
          "id": "cG9zdDoyOTY=",
          "forms": {
            "text": "Отправить сообщение"
          }
        },
        {
          "id": "cG9zdDoyOTU=",
          "forms": {
            "text": "Телефон"
          }
        },
        {
          "id": "cG9zdDoyOTQ=",
          "forms": {
            "text": "Имя"
          }
        },
        {
          "id": "cG9zdDoyOTM=",
          "forms": {
            "text": "Сообщение"
          }
        }
      ]

я делала доступ к данным например через map(el=>el.data) и nodes[index], как получать доступ по другому?

получается что внутри массива все равно приходится делать поиск нужного объекта с данными используя индекс например - nodes[3].id

Copy link
Contributor

Choose a reason for hiding this comment

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

У тебя каждая нода обладает своим ID. Ты ведь можешь их захардкодить и подставлять туда, где ожидаешь инфо по этому ID.

Изменение элемента, его 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.

@Nelfimov

только что исправила и ревью запросила, посмотри как время будет

@@ -25,11 +25,13 @@ export const Card: FC<CardProps> = ({ ...props }) => {
>
{props.image ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

Лучше перейти на <Condition>

@@ -58,7 +58,7 @@ export const Accordion = ({ screen }) => {
fontSize={{ _: 'medium', standard: 'standard', wide: 'major', ultra: 'major' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Переходи выше на Condition


export const Accordion = ({ screen }) => {
const [selected, setSelected] = useState<number | null>(null)
const items = () => (screen === 'wide' ? Array.from({ length: 3 }) : Array.from({ length: 6 }))

const answer = useFaq()
Copy link
Contributor

Choose a reason for hiding this comment

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

В ui хранятся переиспользуемые компоненты. А тут мы зашиваем конкретное значение. Его лучше передавать через пропсы.

@oxiqod oxiqod requested a review from Nelfimov September 6, 2023 09:02
Copy link
Contributor

@Nelfimov Nelfimov left a comment

Choose a reason for hiding this comment

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

Правки + ID нод

@oxiqod oxiqod requested a review from Nelfimov September 6, 2023 13:36
"@makotot/ghostui": "2.0.0",
"@ui/layout": "workspace:0.0.1",
"@ui/text": "workspace:0.0.1",
"graphql": "16.7.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

graphql тоже можно в peer - он отдается у тебя в энтрипоинте

"@emotion/react": "11.11.0",
"@emotion/styled": "11.11.0",
"graphql": "16.7.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Можно в peer

"name": "@ui/condition",
"version": "0.0.1",
"license": "BSD-3-Clause",
"main": "src/index.ts"
Copy link
Contributor

Choose a reason for hiding this comment

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

можно переиспользовать наш - @atls-ui-parts/condition


import { useFaq } from '../data'

export const Feedback = ({ open, onClose }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Интерфейсы

import { Layout } from '@ui/layout'
import { Text } from '@ui/text'
import { useHover } from '@ui/utils'
import { usePressed } from '@ui/utils'

export const Item = ({ ...props }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Интерфейс

{title}
</Text>
</Link>
{path && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Condition сюда хорошо впишется

@oxiqod oxiqod requested a review from Nelfimov September 7, 2023 17:05
@oxiqod oxiqod removed the request for review from TorinAsakura September 7, 2023 18:41
@oxiqod
Copy link
Contributor Author

oxiqod commented Sep 7, 2023

В последнем коммите - поправила то, что забыла ^

Copy link
Member

@TorinAsakura TorinAsakura left a comment

Choose a reason for hiding this comment

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

основные претензии это разного рода вычисления "на лету" вроде {hero?.data?.sections.nodes[6].sections.title.split(' ').slice(0, -1).join(' ')}
и прочих, которые создают нестабильность при работе с данными, то есть, любое смещение в массиве и мы получим не то, что ожидается

@TorinAsakura TorinAsakura merged commit 8725531 into master Sep 10, 2023
4 checks passed
@TorinAsakura TorinAsakura deleted the feat/landing-7 branch September 10, 2023 23:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Знакомство с GraphQL
3 participants