Javascript-форум (https://javascript.ru/forum/)
-   Ваши сайты и скрипты (https://javascript.ru/forum/project/)
-   -   Нужна критика на скрипт выпадающего меню (https://javascript.ru/forum/project/78132-nuzhna-kritika-na-skript-vypadayushhego-menyu.html)

Muvka 31.07.2019 10:49

Нужна критика на скрипт выпадающего меню
 
Написал скрипт для создания выпадающего меню, с использованием БЭМ нотации. И с использованием css анимации. При этом внешний вид каждого меню можно сделать индивидуальным. Но я понимаю, что вот так вот сразу все сделать хорошо не получится. Поэтому пожалуйста, укажите узкие места.
class DropdownMenu {
    constructor(menu) {
      this._menu = menu; // Само меню
      this._button = this._menu.querySelector('.js-dropdown__toggler'); // Кнопка открытия меню
      this._baseClass = this._menu.dataset.baseClass; // Основной класс блока. Для создания правильного класса с --open
      this._isOpen = false; // Открыто меню или нет

      this._button.addEventListener('click', this._toggle.bind(this));

      // Удаление класса закрывающей анимации
      this._menu.addEventListener('animationend', () => {
        this._menu.classList.remove('js-dropdown--close', `${this._baseClass}--close`);
      });
    }

    _toggle(event) {
      if(event) {
        event.preventDefault();
        event.stopPropagation();
      }

      if(this._isOpen) {
        this.close();
      } else {
        // Если есть открытое меню, закрываем его
        if(DropdownMenu.openedMenu) {
          DropdownMenu.openedMenu.close();
        }

        this.open();
      }
    }

    open() {
      this._menu.classList.add('js-dropdown--open');
      
      if(this._baseClass) {
        this._menu.classList.add(`${this._baseClass}--open`);
      }

      this._isOpen = true;
      // Сохраняем открытое меню
      DropdownMenu.openedMenu = this;
    }

    close() {
      this._menu.classList.add('js-dropdown--close');
      this._menu.classList.remove('js-dropdown--open');

      if(this._baseClass) {
        this._menu.classList.add(`${this._baseClass}--close`);
        this._menu.classList.remove(`${this._baseClass}--open`);
      }

      this._isOpen = false;
      DropdownMenu.openedMenu = null;
    }

    static init(menu) {
      if(!DropdownMenu._initialized) {
        DropdownMenu._initialized = true;

        // Закрытие меню при клике вне меню
        document.addEventListener('click', ({target}) => {
          if(DropdownMenu.openedMenu && !target.closest('.js-dropdown__inner')) {
            DropdownMenu.openedMenu.close();
          }
        });
      }

      return new DropdownMenu(menu);
    }
  }

  const dropdownMenus = document.querySelectorAll('.js-dropdown');

  if(dropdownMenus) {
    [...dropdownMenus].forEach((menu) => {
      DropdownMenu.init(menu);
    });
  }

рони 31.07.2019 11:15

Muvka,
может макет полностью сделать?

Malleys 05.08.2019 01:45

Цитата:

Сообщение от рони
может макет полностью сделать?

Пишут ведь, что внешний вид каждого меню можно сделать индивидуальным.

Цитата:

Сообщение от Muvka
Но я понимаю, что вот так вот сразу все сделать хорошо не получится.

Оно у вас уже работает, однако...

Цитата:

Сообщение от Muvka
Поэтому, пожалуйста, укажите узкие места.

Я укажу на некоторые решения, принятые в коде.

Например, почему инициализация частично происходит в конструкторе, а частично в статичном методе? Инициализация должна полностью происходить в конструкторе. Таким образом возможно стандартное создание объекта определённого класса. (Например, new DropdownMenu(menu))

Имена полей, начинающиеся со знака подчёркивания. Иногда говорят, что такие имена указывают на приватные свойства, но на самом деле это не так. Эти свойства могут быть свободно поменяны вне класса! Так что вы можете удалить из имён знаки подчёркивания.

Цитата:

Сообщение от Muvka
this.isOpen = true;
DropdownMenu.openedMenu = this;

Вы указываете, какое меню открыто, и ещё устанавливаете переменную isOpen. Это дубликат. Вы можете, например, оставить только DropdownMenu.openedMenu = this;, а открыто ли конкретное меню, можно узнать, сравнив меню с тем, которое отрыто. Для этого переделайте isOpen в геттер.

Соответственно сеттер isOpen будет открывать или закрывать меню. Для его создания вы можете перенести часть кода из метода toggle.

Цитата:

Сообщение от Muvka
event.stopPropagation();

Действительно ли вам такое необходимо? Что такого особенного в этом простом меню, что нужно блокировать распространение события далее? Хотя такое действительно может быть полезным, когда вы хотите, чтобы событие произошло только на меню! Кнопка открытия меню не совершает никаких действии по умолчанию, поэтому можно удалить и это!

Цитата:

Сообщение от Muvka
this.menu.classList.add("js-dropdown--close");

Поскольку класс js-dropdown--open означает открытое меню, то отсутствие такого класса означает закрытое меню. Соответственно класс js-dropdown--close является лишним!

Теперь this.isOpen = true и this.open(); хотя и открывают меню, но делают это по разному! this.isOpen = true закрывает предыдущее открытое меню, а this.open(); не закрывает. Перенесём определение this.open() в сеттер this.isOpen, чтобы добиться одинакового поведения.

<script>
		
class DropdownMenu {
	constructor(menu) {
		this.menu = menu; // Само меню
		this.button = this.menu.querySelector(".js-dropdown__toggler"); // Кнопка открытия меню
		this.baseClass = this.menu.dataset.baseClass || "js-dropdown"; // Основной класс блока. Для создания правильного класса с --open

		if (!DropdownMenu.initialized) {
			DropdownMenu.initialized = true;

			// Закрытие меню при клике вне меню
			document.addEventListener("click", ({ target }) => {
				if (DropdownMenu.openedMenu && !target.closest(".js-dropdown__inner, .js-dropdown__toggler")) {
					DropdownMenu.openedMenu.close();
				}
			}, true);
		}

		this.button.addEventListener("click", () => {
			this.toggle();
		});
	}

	get isOpen() {
		return DropdownMenu.openedMenu === this;
	}

	set isOpen(isOpen) {
		if(isOpen) {
			if(DropdownMenu.openedMenu)
				DropdownMenu.openedMenu.close();

			this.menu.classList.add("js-dropdown--open", `${this.baseClass}--open`);
			DropdownMenu.openedMenu = this;
		} else {
			this.menu.classList.remove("js-dropdown--open", `${this.baseClass}--open`);
			DropdownMenu.openedMenu = null;
		}
	}

	toggle() {
		this.isOpen ^= true;
	}

	open() {
		this.isOpen = true;
	}

	close() {
		this.isOpen = false;
	}
}

document.addEventListener("DOMContentLoaded", () => {
	for(const menu of document.querySelectorAll(".js-dropdown"))
		new DropdownMenu(menu);
});
	
</script>
<style>
		
:root {
	font: 1em "SF Pro Text", "Helvetica Neue", "Segoe UI", "Ubuntu", sans-serif;
	background: repeating-conic-gradient(deeppink 0 90deg, gold 0 180deg) center / 100px 100px;
}

.js-dropdown button.js-dropdown__toggler {
	margin-top: 1em;
}

.js-dropdown ul.js-dropdown__inner {
	position: absolute;
	border-radius: 0.35em;
	background: rgba(255, 255, 255, 0.8);
	box-shadow: 0 0.5em 1em rgba(0, 0, 0, 0.25);
	margin: 0;
	padding: 0.5em 0;
	list-style: none;
	min-width: 10em;
	backdrop-filter: blur(8px);
	opacity: 0;
	transform: translateY(-8px);
	pointer-events: none;
	transition: 300ms;
}

.js-dropdown ul.js-dropdown__inner li {
	margin: 0;
	padding: 0.5em 1em;
	cursor: default;
}

.js-dropdown ul.js-dropdown__inner li:hover {
	background-color: #1a73e8;
	color: white;
}

.js-dropdown.js-dropdown--open ul.js-dropdown__inner {
	opacity: 1;
	transform: translateY(0);
	pointer-events: all;
}
</style>

<div class="js-dropdown">
	<button class="js-dropdown__toggler">≡ Menu</button>

	<ul class="js-dropdown__inner">
		<li>Rename</li>
		<li>Duplicate</li>
		<li>Delete</li>
	</ul>
</div>

<div class="js-dropdown">
	<button class="js-dropdown__toggler">≡ Menu</button>

	<ul class="js-dropdown__inner">
		<li>Search</li>
		<li>Run command</li>
		<li>Open file</li>
		<li>Settings</li>
		<li>Help</li>
	</ul>
</div>

КостяТТ 09.09.2019 12:02

Могу ли я такой код вставить на свой сайт? Чтобы сильно не париться и сразу сделать выпадающее меню.

Malleys 09.09.2019 13:50

КостяТТ, да! Мой скрипт вы можете вставить на свой сайт!


Часовой пояс GMT +3, время: 19:21.