IwanQ
@IwanQ
Плохие времена часто дают прекрасные возможности

Оцените логику проекта?

Доброго времени суток.

Недавно начал изучать React, мой первый фреймворк. Решил для начала реализовать header.

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

5ffecbdccb85e359303628.png
5ffecbe45ad0a881494791.png
5ffecbead9cfd744953640.png

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

Босс:

/src/hoc/Layout/Layout.js

import { Component } from 'react';
import Header from '../../components/Navigation/Header/Header';

import '../../libs/fontawesome/css/all.min.css';
import cssClasses from './Layout.module.scss';

class Layout extends Component {
  state = {
    isMenuOpen: false,
  }

  toggleMenuHandler = () => {
    const { isMenuOpen } = this.state;
    this.setState({
      isMenuOpen: !isMenuOpen,
    });
  }

  keyPressHandler = ({ key }) => {
    if (key && key === 'Escape') {
      this.toggleMenuHandler();
    }
  }


  render() {
    const { isMenuOpen } = this.state;

    if (isMenuOpen) {
      document.addEventListener('keydown', this.keyPressHandler)
    } else {
      document.removeEventListener('keydown', this.keyPressHandler)
    }

    return(
      <div className={cssClasses.Layout}>
        <Header isMenuOpen={isMenuOpen} toggleMenuHandler={this.toggleMenuHandler} />
        {this.props.children}
      </div>
    );
  }
}

export default Layout;


Далее компоненты:

/src/components/Navigation/Header/Header.js

import Havbar from './Navbar/Navbar';

import cssClasses from './Header.module.scss';

const Header = ({ isMenuOpen, toggleMenuHandler }) => {
  return (
    <header className={cssClasses.Header}>
      <Havbar isMenuOpen={isMenuOpen} toggleMenuHandler={toggleMenuHandler} />
    </header>
  );
};

export default Header;


/src/components/Navigation/Header/Navbar/Navbar.js

import { NavLink, Link } from 'react-router-dom';
import { useSwipeable } from 'react-swipeable';

// UI
import Overlay from '../../../UI/Overlay/Overlay';

import cssClasses from './Navbar.module.scss';
import logo from '../../../../img/logo.jpg';

const links = [
  { to: '/', label: 'Main', exact: true },
  { to: '/faq', label: 'Faq', exact: false },
  { to: '/contacts', label: 'Contacts', exact: false },
];

const navbarLinkGenerator = (linksList, toggleMenuHandler) => {
  return linksList.map(({ to, label, exact }, index) => {
    return (
      <li key={`navlink-${index}`}>
        <NavLink to={to} exact={exact} onClick={toggleMenuHandler} activeClassName={cssClasses.active}>{ label }</NavLink>
      </li>
    );
  });
};

const Havbar = ({ isMenuOpen, toggleMenuHandler }) => {
  const navbarLinksClasses = [cssClasses['Navbar-links']];

  if (isMenuOpen) {
    navbarLinksClasses.push(cssClasses.show);
  }

  const handlers = useSwipeable({
    onSwipedRight: () => toggleMenuHandler(),
  });

  return (
    <>
    <div className="Navbar-logo">
      <div className={cssClasses['Navbar-logo']}>
        <Link to="/">
          <img src={logo} alt="logo" />
        </Link>
      </div>
    </div>

    <div {...handlers} className={navbarLinksClasses.join(' ')}>
    <i className="far fa-times" onClick={toggleMenuHandler}></i>
    <nav>
      <ul>
        {navbarLinkGenerator(links, toggleMenuHandler)}
      </ul>
    </nav>
    </div>

    <div className={cssClasses['Navbar-burger']} onClick={toggleMenuHandler}>
      <i className="fal fa-align-left" />
    </div>
    { isMenuOpen && <Overlay onClick={toggleMenuHandler} /> }
    </>
  );
};

export default Havbar;


/src/components/UI/Overlay/Overlay.js

import classesList from './Overlay.module.scss';

const Overlay = ({ onClick }) => {
  return (
    <div onClick={onClick} className={classesList.Overlay}></div>
  )
};

export default Overlay;


Файлы scss думаю не особо важны.

5ffecddb46045977090495.png

Буду рад услышать, что я сделал не правильно в плане логики и любую другую критику или советы.
Заранее спасибо.
  • Вопрос задан
  • 87 просмотров
Решения вопроса 2
@twolegs
В render такое делать нельзя. Место для такой подписки - componentDidMount. Думаю, правильнее будет подписаться один раз, и не делать ничего, если isMenuOpen в true.
if (isMenuOpen) {
      document.addEventListener('keydown', this.keyPressHandler)
    } else {
      document.removeEventListener('keydown', this.keyPressHandler)
    }


Остальное (организация компонент) - это скорее вкусовщина. Но если смотреть в целом - нет какой-то консистентности. Где-то есть компоненты для логических элементов, где-то нет.
Ответ написан
@n1ksON
Интерн
1. Зачем смешиваете классовые и функциональные компоненты?
2. Вынесите эти участки кода в отдельные файлы
const navbarLinkGenerator = (linksList, toggleMenuHandler) => {
  return linksList.map(({ to, label, exact }, index) => {
    return (
      <li key={`navlink-${index}`}>
        <NavLink to={to} exact={exact} onClick={toggleMenuHandler} activeClassName={cssClasses.active}>{ label }</NavLink>
      </li>
    );
  });
};

const links = [
  { to: '/', label: 'Main', exact: true },
  { to: '/faq', label: 'Faq', exact: false },
  { to: '/contacts', label: 'Contacts', exact: false },
];

3. Лучше переписать так:
было:
const navbarLinkGenerator = (linksList, toggleMenuHandler) => {
  return linksList.map(({ to, label, exact }, index) => {
    return (
      <li key={`navlink-${index}`}>
        <NavLink to={to} exact={exact} onClick={toggleMenuHandler} activeClassName={cssClasses.active}>{ label }</NavLink>
      </li>
    );
  });
};

стало:
const navbarLinkGenerator = (linksList, toggleMenuHandler) => {
  return (
    {linksList.map(({ to, label, exact }, index) => 
      <li key={`navlink-${index}`}>
        <NavLink to={to} exact={exact} onClick={toggleMenuHandler} activeClassName={cssClasses.active}>{ label }</NavLink>
      </li>
  )};
)};

UPD:
4. Лучше переписать с useEffect:
useEffect(() => {
  if (isMenuOpen) {
    navbarLinksClasses.push(cssClasses.show);
  }
}, [isMenuOpen])
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы