Разработка компонента для выпадающего пункта меню

Выполняется

Проект: Пивная карта

Планируемый запуск:

06.05.2021

Дата начала:

07.05.2021

Планируемое завершение:

07.05.2021

Дата завершения:

Проект	Задача	Статус	Дата постановки	Начало	Конец	Кто создал
Пивная карта	Разработка компонента для выпадающего пункта меню	Выполняется	06.05.2021 03:44:15	13.05.2021 15:40:02	13.05.2021 16:18:27	Попов Дмитрий
Пивная карта	Разработка компонента для выпадающего пункта меню	Выполняется	06.05.2021 03:44:15	13.05.2021 04:25:44	13.05.2021 05:20:45	Попов Дмитрий
Пивная карта	Разработка компонента для выпадающего пункта меню	Выполняется	06.05.2021 03:44:15	12.05.2021 16:57:24	12.05.2021 17:39:05	Николай Ланец
Пивная карта	Разработка компонента для выпадающего пункта меню	Выполняется	06.05.2021 03:44:15	09.05.2021 04:19:07	09.05.2021 04:47:51	Попов Дмитрий
Пивная карта	Разработка компонента для выпадающего пункта меню	Выполняется	06.05.2021 03:44:15	07.05.2021 04:21:50	07.05.2021 04:50:23	Попов Дмитрий
Пивная карта	Разработка компонента для выпадающего пункта меню	Выполняется	06.05.2021 03:44:15	07.05.2021 04:04:20	07.05.2021 04:15:24	Попов Дмитрий

Николай, приветствую!

Для оценки правильности поставленной задачи, посмитри, пожалуйста, тезисы. А то может я вообще не правильно понимаю, что делаю))))

Компонент должен принимать:
- название верхнего пункта меню
- список подпунктов выпадающего меню
- отображать все это добро в верхнем меню.

Дима, вот у тебя есть кусок кода: 
        <DropdownMenuBoxStyled>
          <a
            //href={`/city/${coordsUrl}`}
            onClick={toggleMenuCities}
            title="Пивная карта по городам"
          >
            {mainCity.name} <i className="fa fa-angle-down"></i>
          </a>
          <DropdownMenuStyled opened={citiesOpened}>
            {citiesList}
          </DropdownMenuStyled>
        </DropdownMenuBoxStyled>
В нем есть стайлед-компоненты DropdownMenuBoxStyled и DropdownMenuStyled, которые берутся из ./styled.ts

Надо их вынести в отдельный компонент DropdownMenu (там, скорее всего, придется переименовать компоненты DropdownMenuBoxStyled и DropdownMenuStyled, чтобы было логичней, так как сейчас получается, что компонент с более коротким названием является вложенным в компонент с более длинным названием, что противоречит интуитивнопонятному неймингу).

То есть тебе надо создать компонент DropdownMenu, чтобы ты в главном меню мог вызывать так: 
<DropdownMenu
  {...linkProps}
>
  {citiesList}
</DropdownMenu>
linkProps - это я общий набор параметров для ссылки обозначил. Фактически же да, ты будешь перечислять явно типа такого: 
<DropdownMenu
  name={name}
  href={href}
  title={title}
  {/* И т.п. */}
>
  {citiesList}
</DropdownMenu>

А в том компоненте будет типа так: 
const DropdownMenu: React.FC<DropdownMenuProps> = ({
  name,
  href,
  title,
  children,
  ...other
}) => {

  /**
   * Здесь вся логика с citiesOpened, toggleMenuCities и т.п.
   */

  return <DropdownMenuStyled
    {...other}
  >
    <a
      href={href}
      title={title}
      onClick={toggleMenuCities}
    >
      {name} <i className="fa fa-angle-down"></i>
    </a>
    <DropdownSubMenuStyled opened={citiesOpened}>
      {children}
    </DropdownSubMenuStyled>
  </DropdownMenuStyled>;
}
Но здесь, как видишь, other попадает в корневой тег меню, а не в ссылку, и в данной реализации получается, что у тебя ссылка имеет на вход очень ограниченное число параметров, хотя может быть сильно больше, чем ты изначально предполагаешь. Возможно имеет смысл href, title, name вынести в отдельный параметр типа linkProps, чтобы можно было сделать так: 
const DropdownMenu: React.FC<DropdownMenuProps> = ({
  linkProps: {
    name,
    href,
    title,
    ...otherLinkProps
  },
  children,
  ...other
}) => {

  /**
   * Здесь вся логика с citiesOpened, toggleMenuCities и т.п.
   */

  return <DropdownMenuStyled
    {...other}
  >
    <a
      href={href}
      title={title}
      onClick={toggleMenuCities}
      {...otherLinkProps}
    >
      {name} <i className="fa fa-angle-down"></i>
    </a>
    <DropdownSubMenuStyled opened={citiesOpened}>
      {children}
    </DropdownSubMenuStyled>
  </DropdownMenuStyled>;
}

Тогда у тебя получится более гибкий компонент, особенное если ты найдешь способ параметру linkProps задать одной конструкцией все возможные свойства ссылок:-) (Это тебе дополнительное субзадание, которое попытайся решить, но если не получится, то не фанатей).

Спасибо! Разбираюсь)

Николай, привет! Несмотря на то, что ты все описал подробно, я тыкаю кодом компонента в разные места и довольно бездумно. 
Компонент:

const DropdownMenu: React.FC<DropdownMenuProps> = ({
    name,
    href,
    title,
    children,
    ...other
  }) => {
  
    /**
     * Здесь вся логика с citiesOpened, toggleMenuCities и т.п.
     */
  
     const [citiesOpened, citiesOpenedSetter] = useState(false)
  
     const toggleMenuCities = useCallback(() => {
       citiesOpenedSetter(!citiesOpened)
     }, [citiesOpened])
   
     useEffect(() => {
       if (!citiesOpened) {
         return
       }
   
       const closeCitiesOpenedEvent = () => {
         citiesOpenedSetter(false)
       }
   
       window.document.addEventListener('click', closeCitiesOpenedEvent)
   
       return () => {
         window.document.removeEventListener('click', closeCitiesOpenedEvent)
       }
     }, [citiesOpened])
   
     const citiesList = useMemo<JSX.Element | null>(() => {
       //const coordsUrl = ''
   
       const citiesList: JSX.Element[] = []
   
       cities.forEach((city) => {
         if (!city) {
           return
         }
   
         const { id, name, lat, lng, alias: uri } = city
   
         if (!lat || !lng) {
           return
         }
   
         const link = `/${uri}@` + [lat, lng, 12].join(',')
   
         citiesList.push(
           <li key={id}>
             <Link href={link}>
               <a
                 style={{
                   color: '#000',
                 }}
   
                 // onClick={closeMenu}
               >
                 {name}
               </a>
             </Link>
           </li>
         )
       })
   
  
    return <DropdownMenuStyled
      {...other}
    >
      <a
        href={href}
        title={title}
        onClick={toggleMenuCities}
      >
        {name} <i className="fa fa-angle-down"></i>
      </a>
      <DropdownMenuBoxStyled opened={citiesOpened}>
        {children}
      </DropdownMenuBoxStyled>
    </DropdownMenuStyled>
    },[])

Засунул его перед компонентом MainMenu - ругается, что export MainMenu не катит - можно только элемент верхнего уровня экспортировать, внутрь компонента MainMenu перенес - ошибка та же. Вообще вынести в отдельный файл всё это хозяйство надо?

Привет!

Ты явно перенес больше, чем следовало. Откуда у тебя здесь переменная cities? Ты же ее в пропсы не передавал. 
Выливай что есть в отдельную ветку, буду посмотреть.

Николай, привет!
https://github.com/Pivkarta/pivkarta.ru-2/commit/09bfaee950457d7a15aeb1f4a514760b8c4bfb4a

Посмотришь по возможности?

Дима, привет!

Сейчас гляну.

А у тебя разве вот такой ошибки нет? 

У тебя там сплошная синтаксическая ошибка, у тебя код вообще не валидный (у тебя блок {...} не закрывается корректно).

И вот еще у тебя ошибки:
$ tsc --pretty --noEmit
src/pages/_App/Layout/MainMenu/index.tsx:27:7 - error TS2322: Type '() => void' is not assignable to type 'FC<{}>'.
  Type 'void' is not assignable to type 'ReactElement<any, any> | null'.

27 const MainMenu: React.FC = () => {
         ~~~~~~~~

src/pages/_App/Layout/MainMenu/index.tsx:137:32 - error TS2304: Cannot find name 'DropdownMenuProps'.

137   const DropdownMenu: React.FC<DropdownMenuProps> = ({
                                   ~~~~~~~~~~~~~~~~~

src/pages/_App/Layout/MainMenu/index.tsx:217:32 - error TS2769: No overload matches this call.

Разве ты у себя их не видишь?

И почему ты DropdownMenu объявил в том же файле, а не в отдельном? 

А вот тут ты вообще очень косо въехал, вызывая <DropdownMenu /> внутри DropdownMenu. Хорошо хоть ты его нигде во внешнем не прописал (явно забыл), иначе ушел бы в бесконечную рекурсию.

Спасибо, изучаю!

В общем, ты пока явно плохо видишь границы реакт-компонентов. Напомню: у нас функциональное программирование и реакт-компонент сейчас - это просто функция. 

Редактировать то, что у тебя сейчас сделано - смысла мало. Там каша получилась. Вот ПР отправил: https://github.com/Pivkarta/pivkarta.ru-2/pull/3. Принимай и разбирай (что и как работает).

Так же оставил тебе задачку: 
>>  (там, скорее всего, придется переименовать компоненты DropdownMenuBoxStyled и DropdownMenuStyled, чтобы было логичней, так как сейчас получается, что компонент с более коротким названием является вложенным в компонент с более длинным названием, что противоречит интуитивнопонятному неймингу)

Вот перемеименуй компоненты и шли коммит на ревью.

Николай, привет!

Спасибо: изучаю код. Когда он есть - то всё понятно, а когда сам пишкшь - всё как-то меняется...

Коммит на изменение названия компонента: https://github.com/Pivkarta/pivkarta.ru-2/commit/7014b69d973ea8806587f3313905dbb9aa2ebd12

Дима, привет!

>> Когда он есть - то всё понятно, а когда сам пишкшь - всё как-то меняется...

Так ты смотри в то, что уже есть и что понятно, и делай по образу и подобию. Ведь общая структура типовая (index.tsx, interfaces.ts, styles.ts). И я ранее писал уже про то, чтобы стараться "Один файл - Одна сущность", то есть чтобы реакт-компонент в файле был только один. Ты забыл про это правило и дописал свой новый компонент в тот же файл к существующему компоненту. Скорее всего, если бы ты придержался этого правила, тебе было бы проще и меньше ошибок допустил был (потому что перед глазами меньше кода было бы и проще было бы видно границы). 

И ты так и не ответил на счет ошибок, я несколько сообщений на этот счет написал. Мне не понятно видишь ты их или нет. Одно дело логику придумывать, другое дело решать чисто синтаксические ошибки типа "Здесь неожиданный тег". Ты очень многое не договариваешь.

Каюсь, ошибки видел. Но не разобрался, на что ругался... 
Но проблема не в них, а в том, что понимания компонента не было. Сейчас понял. Вроде бы.
Спасибо.

И зря избавился от DropdownMenuStyled. Я ранее тоже писал, что по неймингу лучше писать так: к имени компонента добавлять Styled, для стайлед-компонентов. Если у тебя компонент DropdownMenu, то для корневого элемента лучше заюзать DropdownMenuStyled, а вот внутри уже можешь что-то добавить еще (как тот же DropdownMenuListStyled (но не DropdownMenuBoxListStyled)). У тебя же сейчас получается
const DropdownMenuBoxListStyled = styled.ul
const DropdownMenuBoxStyled = styled.li
А я выше писал:
>> так как сейчас получается, что компонент с более коротким названием является вложенным в компонент с более длинным названием, что противоречит интуитивнопонятному неймингу

То есть ты переименовал, но пришел все к тому же: более короткое вложено в более длинное (к слову, я это заметил, только открыв исходники, хотя изначально по наименованию подумал про обратную вложенность). Плюс к этому у тебя DropdownMenuBoxStyled - это тег li и у тебя их много. Получается, у тебя много боксов в одном месте. Согласись, опять не интуитивно понятно. Я же советую так:
DropdownMenuStyled - это корневой ui,
DropdownMenuItemStyled - это li.

Поправь.

>> Каюсь, ошибки видел. Но не разобрался, на что ругался... 
Но проблема не в них, а в том, что понимания компонента не было.

Нет, проблема как раз в них. А точнее в том, что ты умалчиваешь за эти проблемы. Еще раз: одно дело писать логику, придумывать ее. Другое дело - решать чисто технические синтаксические задачи. Если ты видишь синтаксическую ошибку (про которую тебе подсказывает IDE), ты ее должен решить обязательно. Если ты не можешь ее решить, ты спрашиваешь как именно решить конкретно эту ошибку. Это чисто типовой подход. Нет никакого смысла пытаться написать какую-то логику, если у тебя имеются технические ошибки, из-за которых все равно ничего работать не будет. Если у тебя не работает логика, но технически все работает - это уже пол-дела. А если нет ни того, ни другого - то это полная бессмыслица. Работай над этим.

А так ты сейчас просто как страус прячешь голову в песок.

Принято, исправляюсь

Чего-то я запутался с переименованиями: какой нейминг правильный?
Более высокий элемент - более короткий в названии?

То есть:
<CompStyled>
      <CompElementStyled>
                  <CompElementSubelementStyled>
         
                  </CompElementSubelementStyled> 
      </CompElementStyled> 
</CompStyled> 

Или как-то иначе?

Да, так. Только CompStyled - это если компонент Comp (то есть ИмяКомпонента + Styled).
У тебя DropdownMenu, соответственно DropdownMenuStyled.

Коммит: https://github.com/Pivkarta/pivkarta.ru-2/commit/72f1bd069c4e201f2253426c000e42523b8470ab

Посмотришь?

А это что за рудимент?

Ага, прилетел из прошлого коммита: начал заводить компонент. Как можно убрать с гита, не подскажешь?

Не поверишь: удалить файл (директорию) и закоммитить :)

:)))

Так я удалил до формирования коммита, то есть должно было залететь, Но нет

Удалить мало. Надо еще сделать git add ., чтобы добавить удаление в коммит. Иначе в историю ничего не попадет (сам факт удаления не запишется). Тебе git status должен был показать удаленные файлы.

Отправил ПР. 

>> DropdownMenuStyled - это корневой ui,
DropdownMenuItemStyled - это li.

Тут я спутал. Все наоборот. Поэтому переименовал DropdownMenuItemStyled в DropdownSubmenuStyled, ибо это ul.

Кстати, обрати внимание, что вот это можно было и не писать. 
  ${({ opened }) => {
    if (opened) {
      return css`
        display: block;
      `
    } else {
      return css`
        display: none;
      `
    }
  }}

Можно было вместо этого 
<DropdownMenuItemStyled opened={citiesOpened}>
  {children}
</DropdownMenuItemStyled>
написать так: 
{citiesOpened ? 
  <DropdownMenuItemStyled>
    {children}
  </DropdownMenuItemStyled> : 
  null
}
То есть если citiesOpened есть (в нашем случае === true), то выводить субменю. Если нет, то не выводить. Тогда и в стилях нам не надо рулить с display: none; (ну нет же выводимых элементов, значит и скрывать ничего не надо). И это другой способ реализации твоей задачи, при чем проще и довольно распространенный. Но здесь есть большое и важное отличие, почему я именно такой и показал. В первом случае (как у нас сейчас сделано), можно добавить всякие анимационные штучки (типа плавного открытия/закрытия и т.п.), потому что у нас субменю всегда есть, только атрибуты у него меняются. Во втором же случае субменю то есть, то нету, то есть она в пограничные моменты мгновенно появляется и мгновенно исчезает. В таком случае добавить какую-либо аниманию очень сложно становится. Но это не означает, что второй вариант не годится вообще. В тех случаях, где анимация и прочие такие вещи не нужны, второй вариант предпочтительней даже, потому что мокращает объем DOM, ведь в первом случае мы всегда отрисовываем кучу li-элементов (просто прячем за display: none), а во втором случае элементов нет вообще, что снижает нагрузку на рендеринг. Пометь себе это как-нибудь. 

Да, если только есть-нет элемента, то правильнее второй вариант.

>>Удалить мало. Надо еще сделать git add ., чтобы добавить удаление в коммит. Иначе в историю ничего не попадет (сам факт удаления не запишется). Тебе git status должен был показать удаленные файлы.
Так я сначала удалил, а потом уже за коммит принялся. Но не суть. А суть в том ,что согласно замечаниям ,сначала yarn & yarn types сделал, который и нашел мне этот рудимент))  

В итоге, смог удалить и закоммитить?

Рудимента в мастере нет. Я так понимаю - он остался в test3. Это изменение залетит, когда следующий коммит туда будет послан (если будет) 

Аа, ну ОК, вероятнее всего.

FreeCode.Academy

Разработка компонента для выпадающего пункта меню

Добавить комментарий