Javascript-форум (https://javascript.ru/forum/)
-   Events/DOM/Window (https://javascript.ru/forum/events/)
-   -   подменю при клике на элемент меню, насколько правильно написано (https://javascript.ru/forum/events/82663-podmenyu-pri-klike-na-ehlement-menyu-naskolko-pravilno-napisano.html)

13Foch 08.06.2021 17:28

подменю при клике на элемент меню, насколько правильно написано
 
<style>
		.menu__item {
			position: relative;
			display: inline-block;
			margin: 0 10px;
			cursor: pointer;
		}
		.menu__item.menu__item--active {
			background-color: silver;
		}
		.menu__menu-lvl2 {
			position: absolute;
			display: none;
			top: 100%;
			left: 0;
			box-shadow: 1px 1px 30px 1px silver;
		}
		.menu__menu-lvl2.menu__menu-lvl2--active {
			display: block;
			color: red;
		}
	</style>

	<ul class="menu">
		<li class="menu__item">
			<div class="menu__name">item1</div>
			<ul class="menu__menu-lvl2">
				<li>item1__lvl2__item1</li>
				<li>item1__lvl2__item2</li>
				<li>item1__lvl2__item3</li>
			</ul>
		</li>
		<li class="menu__item">
			<div class="menu__name">item2</div>
			<ul class="menu__menu-lvl2">
				<li>item2__lvl2__item1</li>
				<li>item2__lvl2__item2</li>
				<li>item2__lvl2__item3</li>
			</ul>
		</li>
		<li class="menu__item">
			<div class="menu__name">item3</div>
			<ul class="menu__menu-lvl2">
				<li>item3__lvl2__item1</li>
				<li>item3__lvl2__item2</li>
				<li>item3__lvl2__item3</li>
			</ul>
		</li>
		<li class="menu__item">
			<div class="menu__name">item4</div>
		</li>
	</ul>

let menuItems = document.querySelectorAll('.menu__item');
		for (let menuItem of menuItems) {
			menuItem.onclick = function (evt) {
				evt.preventDefault();
				for (let menuItemRemove of menuItems) {
					if (menuItemRemove !== this) {
						menuItemRemove.classList.remove('menu__item--active');
						let menuItemRemoveExists = menuItemRemove.querySelector('.menu__menu-lvl2');
						if (menuItemRemoveExists) {
							menuItemRemoveExists.classList.remove('menu__menu-lvl2--active');
						}
					}
				}
				this.classList.toggle('menu__item--active');
				// console.log(this.parentElement.parentElement);
				if (this.querySelector('.menu__menu-lvl2')) {
					this.querySelector('.menu__menu-lvl2').classList.toggle('menu__menu-lvl2--active');
				}
			}
		}


Подскажите насколько правильно написан js и насколько можно его сократить. Спасибо

ksa 08.06.2021 18:55

Цитата:

Сообщение от 13Foch
насколько правильно написан js и насколько можно его сократить

Например можно делать вообще один клик на все меню разом.
Там все и решать.
А у тебя довольно много циклов.

рони 08.06.2021 19:49

13Foch,
<!DOCTYPE html>
<html>
<head>
    <title>Untitled</title>
    <meta charset="utf-8">
</head>
<body>
    <style>
        .menu__item {
            position: relative;
            display: inline-block;
            margin: 0 10px;
            cursor: pointer;
        }
        .menu__item.menu__item--active {
            background-color: silver;
        }
        .menu__menu-lvl2 {
            position: absolute;
            display: none;
            top: 100%;
            left: 0;
            box-shadow: 1px 1px 30px 1px silver;
        }
        .menu__item.menu__item--active>.menu__menu-lvl2 {
            display: block;
            color: red;
        }
    </style>
    <ul class="menu">
        <li class="menu__item">
            <div class="menu__name">item1</div>
            <ul class="menu__menu-lvl2">
                <li>item1__lvl2__item1</li>
                <li>item1__lvl2__item2</li>
                <li>item1__lvl2__item3</li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item2</div>
            <ul class="menu__menu-lvl2">
                <li>item2__lvl2__item1</li>
                <li>item2__lvl2__item2</li>
                <li>item2__lvl2__item3</li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item3</div>
            <ul class="menu__menu-lvl2">
                <li>item3__lvl2__item1</li>
                <li>item3__lvl2__item2</li>
                <li>item3__lvl2__item3</li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item4</div>
        </li>
    </ul>
    <script>
        let current;
        document.querySelector(".menu").addEventListener("click", function(event) {
            let target = event.target;
            if (target = target.closest(".menu__name")) {
                event.preventDefault();
                let parent = target.parentNode;
                if (current && current !== parent) current.classList.remove("menu__item--active");
                current = parent;
                current.classList.toggle("menu__item--active");
            }
        });
    </script>
</body>
</html>

13Foch 08.06.2021 20:13

Цитата:

Сообщение от ksa (Сообщение 537776)
Например можно делать вообще один клик на все меню разом.
Там все и решать.
А у тебя довольно много циклов.

второй цикл для того чтоб все остальные закрылись и проверка if для того чтобы при клике на тот же пункт подменю закрылось

13Foch 08.06.2021 20:19

Цитата:

Сообщение от рони (Сообщение 537778)
13Foch,
<!DOCTYPE html>
<html>
<head>
    <title>Untitled</title>
    <meta charset="utf-8">
</head>
<body>
    <style>
        .menu__item {
            position: relative;
            display: inline-block;
            margin: 0 10px;
            cursor: pointer;
        }
        .menu__item.menu__item--active {
            background-color: silver;
        }
        .menu__menu-lvl2 {
            position: absolute;
            display: none;
            top: 100%;
            left: 0;
            box-shadow: 1px 1px 30px 1px silver;
        }
        .menu__item.menu__item--active>.menu__menu-lvl2 {
            display: block;
            color: red;
        }
    </style>
    <ul class="menu">
        <li class="menu__item">
            <div class="menu__name">item1</div>
            <ul class="menu__menu-lvl2">
                <li>item1__lvl2__item1</li>
                <li>item1__lvl2__item2</li>
                <li>item1__lvl2__item3</li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item2</div>
            <ul class="menu__menu-lvl2">
                <li>item2__lvl2__item1</li>
                <li>item2__lvl2__item2</li>
                <li>item2__lvl2__item3</li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item3</div>
            <ul class="menu__menu-lvl2">
                <li>item3__lvl2__item1</li>
                <li>item3__lvl2__item2</li>
                <li>item3__lvl2__item3</li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item4</div>
        </li>
    </ul>
    <script>
        let current;
        document.querySelector(".menu").addEventListener("click", function(event) {
            let target = event.target;
            if (target = target.closest(".menu__name")) {
                event.preventDefault();
                let parent = target.parentNode;
                if (current && current !== parent) current.classList.remove("menu__item--active");
                current = parent;
                current.classList.toggle("menu__item--active");
            }
        });
    </script>
</body>
</html>

уже короче и идея немного другая спасибо

ksa 08.06.2021 20:20

Цитата:

Сообщение от 13Foch
для того чтоб все остальные закрылись

Если я правильно понял, "открытым" у тебя может быть только один.
Т.ч. и закрывать нужно один. Какой тут цикл?

13Foch 08.06.2021 20:23

Цитата:

Сообщение от ksa (Сообщение 537781)
Если я правильно понял, "открытым" у тебя может быть только один.
Т.ч. и закрывать нужно один. Какой тут цикл?

да и закрываться должен он если снова по нему клацнуть

рони 08.06.2021 20:24

13Foch,
не копируйте пост целиком без необходимости, это засоряет тему, есть цитирование.

ksa 08.06.2021 20:25

Цитата:

Сообщение от 13Foch
да и закрываться должен он если снова по нему клацнуть

Значит нет никаких циклов совсем...

Что и показал рони в своем примере.

13Foch 08.06.2021 20:26

ок извиняюсь

рони 08.06.2021 20:26

13Foch,
на всякий случай, оба варианта и ваш и мой не рассчитаны на большее вложение.

13Foch 08.06.2021 20:29

Цитата:

Сообщение от ksa
Значит нет никаких циклов совсем

получаться так спасибо

13Foch 08.06.2021 20:32

Цитата:

Сообщение от рони
на всякий случай, оба варианта и ваш и мой не рассчитаны на большее вложение.

пока что сделаю так а потом как столкнусь буду думать

13Foch 08.06.2021 20:39

Цитата:

Сообщение от рони
на всякий случай, оба варианта и ваш и мой не рассчитаны на большее вложение.

или у вас есть какая то функция которая подходит на все случаи ?

рони 08.06.2021 20:52

13Foch,
примерно так, убираем класс на соседних одного уровня.
<!DOCTYPE html>
<html>
<head>
    <title>Untitled</title>
    <meta charset="utf-8">
</head>
<body>
    <style>
        .menu__item {
            position: relative;
            display: inline-block;
            margin: 0 10px;
            cursor: pointer;
        }
        .menu__item.menu__item--active {
            background-color: silver;
        }
        .menu__menu-lvl2 {
            position: absolute;
            display: none;
            top: 100%;
            left: 0;
            box-shadow: 1px 1px 30px 1px silver;
        }
        .menu__item.menu__item--active>.menu__menu-lvl2 {
            display: block;
            color: red;
             background-color: rgba(255, 255, 255, 1);
        }
        ul{
            padding: 0;
        }
        li{
            list-style: none;
        }

    </style>
    <ul class="menu">
        <li class="menu__item">
            <div class="menu__name">item1</div>
            <ul class="menu__menu-lvl2">
                <li class="menu__item"><div class="menu__name">item1.1</div>
            <ul class="menu__menu-lvl2">
                <li>item1__lvl2__item1</li>
                <li>item1__lvl2__item2</li>
                <li>item1__lvl2__item3</li>
            </ul></li>
                <li class="menu__item"><div class="menu__name">item1.2</div>
            <ul class="menu__menu-lvl2">
                <li>item1__lvl2__item1</li>
                <li>item1__lvl2__item2</li>
                <li>item1__lvl2__item3</li>
            </ul></li>
                <li class="menu__item"><div class="menu__name">item1.3</div>
            <ul class="menu__menu-lvl2">
                <li>item1__lvl2__item1</li>
                <li>item1__lvl2__item2</li>
                <li>item1__lvl2__item3</li>
            </ul></li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item2</div>
            <ul class="menu__menu-lvl2">
                <li>item2__lvl2__item1</li>
                <li>item2__lvl2__item2</li>
                <li>item2__lvl2__item3</li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item3</div>
            <ul class="menu__menu-lvl2">
                <li>item3__lvl2__item1</li>
                <li>item3__lvl2__item2</li>
                <li>item3__lvl2__item3</li>
            </ul>
        </li>
        <li class="menu__item">
            <div class="menu__name">item4</div>
        </li>
    </ul>
    <script>
        document.querySelector(".menu").addEventListener("click", function(event) {
            let target = event.target;
            if (target = target.closest(".menu__name")) {
                event.preventDefault();
                let parent = target.parentNode;
                let children = parent.parentNode.children;
                [...children].forEach(el => el.classList[ el === parent ? "toggle" : "remove"]("menu__item--active"))
            }
        });
    </script>
</body>
</html>

13Foch 08.06.2021 21:15

Цитата:

Сообщение от рони
примерно так, убираем класс на соседних одного уровня.

понял спасибо еще раз

13Foch 08.06.2021 21:19

а почему мало используете this, смотрю мало его люди используют а мне он очень нравиться, я почти везде где только можно суну this )

ksa 08.06.2021 21:40

Цитата:

Сообщение от 13Foch
а почему мало используете this, смотрю мало его люди используют

Ты напомнил мне старый анекдот... :D

На светофоре стоит Ламборджини... Рядом останавливается Логан.
Водила Логана делает знаки водиле Ламборджини, де есть вопрос к нему... Водила Ламборджини опускает стекло:
- Что хотел?
- Хотел узнать как тачка?
- Нормальная.
- Не ломается?
- Нет. А в чем проблема?
- Да вот смотрю не особо ее народ покупает...

рони 08.06.2021 21:43

Цитата:

Сообщение от 13Foch
а почему мало используете this,

куда бы его засунуть тут, не подскажите? )))

13Foch 08.06.2021 21:54

Цитата:

Сообщение от ksa
Ты напомнил мне старый анекдот...

:)

13Foch 08.06.2021 22:10

Цитата:

Сообщение от рони
куда бы его засунуть тут, не подскажите? )))

да тут никуда не всунешь потому что this это .menu а не .menu__item

13Foch 08.06.2021 22:19

Цитата:

Сообщение от ksa
На светофоре стоит Ламборджини... Рядом останавливается Логан.
Водила Логана делает знаки водиле Ламборджини, де есть вопрос к нему... Водила Ламборджини опускает стекло:
- Что хотел?
- Хотел узнать как тачка?
- Нормальная.
- Не ломается?
- Нет. А в чем проблема?
- Да вот смотрю не особо ее народ покупает...

я понимаю мой код для вас глупым кажется )), но спасибо мне полезна любая информация негативная тоже пока катаюсь на логане )

ksa 08.06.2021 22:22

13Foch, просто всему свое место... Так и this используется там, где он нужен.

13Foch 08.06.2021 22:25

Цитата:

Сообщение от ksa
13Foch, просто всему свое место... Так и this используется там, где он нужен.

Понятно

13Foch 08.06.2021 22:34

Цитата:

Сообщение от рони
куда бы его засунуть тут, не подскажите? )))

благодаря вашему вопросу посмотрел как работает target и на этом спасибо раньше не знал

рони 08.06.2021 22:35

13Foch,
:victory:

13Foch 08.06.2021 22:46

let a = document.querySelector(".menu");
		if (a) {
			a.addEventListener("click", function (event) {
				let target = event.target;
				if (target = target.closest(".menu__name")) {
					event.preventDefault();
					let parent = target.parentNode;
					let children = parent.parentNode.children;
					[...children].forEach(el => el.classList[el === parent ? "toggle" : "remove"]("menu__item--active"))
				}
			});
		}


а так нормально писать ? а то вдруг на следующей странице не будет меню этого получиться ошибка

рони 08.06.2021 22:53

Цитата:

Сообщение от 13Foch
а так нормально писать ?

да

рони 08.06.2021 22:59

Цитата:

Сообщение от 13Foch
а то вдруг на следующей странице не будет меню этого получиться ошибка

можно клик не на меню ставить в данном случае а на document, и тогда никаких проверок.
но ваш код более правильный, это приём делегирование, лучше делать на ближайший общий предок и не грузить лишние обработчики.

13Foch 08.06.2021 23:04

понял

13Foch 08.06.2021 23:23

let a = function (b) {
			document.querySelector(b).addEventListener("click", function (event) {
				let target = event.target;
				if (target = target.closest(".menu__name")) {
					event.preventDefault();
					let parent = target.parentNode;
					let children = parent.parentNode.children;
					[...children].forEach(el => el.classList[el === parent ? "toggle" : "remove"]("menu__item--active"))
				}
			});
		}
		a('.menu');

а так более правильно ?

рони 09.06.2021 00:19

13Foch,
а если нет элемента?

13Foch 09.06.2021 09:08

Цитата:

Сообщение от рони
а если нет элемента?

да

13Foch 09.06.2021 09:31

и переменные будут в не зоны видимости их не переназначат

рони 09.06.2021 09:41

13Foch,
:-?
document.addEventListener( "DOMContentLoaded" , function() {
const menuToggle = function (selector) {
            const menu = document.querySelector(selector);
            if(!menu) return;
            menu.addEventListener("click", function (event) {
                let target = event.target;
                if (target = target.closest(".menu__name")) {
                    event.preventDefault();
                    let parent = target.parentNode;
                    let children = parent.parentNode.children;
                    [...children].forEach(el => el.classList[el === parent ? "toggle" : "remove"]("menu__item--active"))
                }
            });
        }
menuToggle('.menu');
  });

13Foch 09.06.2021 09:44

понял спасибо


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