Форум

13Foch · #1 (**permalink**) 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 · #2 (**permalink**) 08.06.2021, 18:55

Сообщение от 13Foch

насколько правильно написан js и насколько можно его сократить

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

рони · #3 (**permalink**) 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 · #4 (**permalink**) 08.06.2021, 20:13

Сообщение от ksa

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

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

13Foch · #5 (**permalink**) 08.06.2021, 20:19

Сообщение от рони

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 · #6 (**permalink**) 08.06.2021, 20:20

Сообщение от 13Foch

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

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

13Foch · #7 (**permalink**) 08.06.2021, 20:23

Сообщение от ksa

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

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

рони · #8 (**permalink**) 08.06.2021, 20:24

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

ksa · #9 (**permalink**) 08.06.2021, 20:25

Сообщение от 13Foch

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

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

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

13Foch · #10 (**permalink**) 08.06.2021, 20:26

ок извиняюсь

Форум

Справочник

Discord чат

Курсы javascript