.header__nav-btn 、.cover-darken 、.js-open の3つの要素を取得後に
.header__nav-btnと.cover-darkenをクリックするとjs-openにis-openedという
クラスが付与される関数を作りました。
同じようなコードを何度も書いており、もっとスマートな書き方が出来そうですがアイデアが浮かびません。
このような場合どのような書き方が適しているのでしょうか?
よろしくお願いします。
js
1function menu_open(str1, str2, str3) { 2 const nav_btn = document.querySelector(str1); 3 const cover_darken = document.querySelector(str2); 4 const js_open = document.querySelector(str3); 5 6 nav_btn.addEventListener("click", function () { 7 js_open.classList.toggle("is-opened"); 8 }); 9 cover_darken.addEventListener("click", function () { 10 js_open.classList.toggle("is-opened"); 11 }); 12 } 13 menu_open(".header__nav-btn", ".cover-darken", ".js-open");
気になる質問をクリップする
クリップした質問は、後からいつでもMYページで確認できます。
またクリップした質問に回答があった際、通知やメールを受け取ることができます。
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
回答3件
0
class="is-opened"
"is-opened" という名前にはかなり違和感があります。
isは等価性を表していますが、「要素 = 開かれている」は成立しません。
例えば、input要素にはdisabled属性があるので、開かれている状態を表すなら、class="opened" で良いのではないかと思います。
スマートな書き方を追求する為にわかりやすい名前を考えてみてください。
スマートな書き方
「スマートなコードの書き方」は要件次第です。
次の要件を満たすのであれば、querySelectorAll
でクリックされるボタン要素をまとめる方法が考えられます。
- 「class="js-open"」な要素が文書内に1個
そして、次の改善を加えると、
- clickイベントハンドラ関数を2回定義している部分を「1回定義して変数にキャッシュ」に変更
- 「class="js-open"」の要素をclickする度に検索している部分を「変数にキャッシュ」に変更
下記コードになります。
HTML
1<style> 2 .opened { 3 font-weight: bold; 4 color: black; 5 background-color: #fee; 6 border: solid 1px red; 7 } 8</style> 9 10<div class="js-open">.js-open</div> 11<div> 12 <input type="button" class="header__nav-btn" value="js-openを開く/閉じる"> 13 <input type="button" class="cover-darken" value="js-openを開く/閉じる"> 14</div> 15<div> 16 <input type="button" class="header__nav-btn" value="js-openを開く/閉じる"> 17 <input type="button" class="cover-darken" value="js-openを開く/閉じる"> 18</div> 19 20<script> 21 'use strict'; 22 23 function addMenuOpenEventBySelectorText(clickBtnSelector, toggleElmSelector) { 24 const listener = { 25 toggleClassList: document.querySelector(toggleElmSelector).classList, 26 handleEvent: function handleClick() { 27 this.toggleClassList.toggle('opened'); 28 } 29 } 30 31 for (let clickBtn of document.querySelectorAll(clickBtnSelector)) { 32 clickBtn.addEventListener('click', listener, false); 33 } 34 } 35 36 addMenuOpenEventBySelectorText(['.header__nav-btn','.cover-darken'], '.js-open'); 37</script>
querySelectorAll()
の第一引数に配列を渡しても動作するのは、querySelectorAll() が内部的にString型への変換を行っており、変換後の文字列が「カンマ区切りのセレクタ文字列」になる為です。
「カンマ区切りのセレクタ文字列」('.header__nav-btn,.cover-darken'
)を渡しても同じ結果となりますが、質問文のコードはクラスセレクタを個別に管理していた為、ここでは配列としました。
初めに下記要件を定義しましたが、これは「class="js-open"」な要素が複数存在した場合、querySelector('js-opened')
によって初めの一つにのみ反応するからです。
- 「class="js-open"」な要素が文書内に1個
「class="header__nav-btn"」「class="cover-darken"」な要素は質問文のコードと違い、上記コードでは複数存在しても反応します。
class属性は文書上に複数存在できる為、単体を求める場合にはid属性に変更する事を検討しましょう。
id属性に変更した参考コードが下記になります。
HTML
1<style> 2 .opened { 3 font-weight: bold; 4 color: black; 5 background-color: #fee; 6 border: solid 1px red; 7 } 8</style> 9 10<div id="content-a">content-a</div> 11<div> 12 <input type="button" class="header__nav-btn" value="content-aを開く/閉じる"> 13 <input type="button" class="cover-darken" value="content-aを開く/閉じる"> 14</div> 15<div> 16 <input type="button" class="header__nav-btn" value="content-aを開く/閉じる"> 17 <input type="button" class="cover-darken" value="content-aを開く/閉じる"> 18</div> 19 20<script> 21 'use strict'; 22 23 function addMenuOpenEventBySelectorText(clickBtnSelector, toggleElmSelector) { 24 const listener = { 25 toggleClassList: document.querySelector(toggleElmSelector).classList, 26 handleEvent: function handleClick() { 27 this.toggleClassList.toggle('opened'); 28 } 29 } 30 31 for (let clickBtn of document.querySelectorAll(clickBtnSelector)) { 32 clickBtn.addEventListener('click', listener, false); 33 } 34 } 35 36 addMenuOpenEventBySelectorText('input[value="content-aを開く/閉じる"]', '#content-a'); 37</script>
先程とほとんどコードは変わりませんが、より洗練されたコードになったと思います。
- HTMLを
<div id="content-a">
に変更した事でtoggle対象要素が一つしか存在できなくなりました - clickボタン要素のセレクタを
'input[value="content-aを開く/閉じる"]'
に変更する事でセレクタがすっきりしました
まとめると、コードを書く前にスマートな要件定義を行ってください。
汎用性
イベントリスナ定義も関数を外に出すと、下記コードになります。
<style> .opened { font-weight: bold; color: black; background-color: #fee; border: solid 1px red; } </style> <div id="content-a">content-a</div> <div> <input type="button" class="header__nav-btn" value="content-aを開く/閉じる"> <input type="button" class="cover-darken" value="content-aを開く/閉じる"> </div> <div> <input type="button" class="header__nav-btn" value="content-aを開く/閉じる"> <input type="button" class="cover-darken" value="content-aを開く/閉じる"> </div> <script> 'use strict'; function addEventListeners(currentTargets, type, listener, options) { for (let currentTarget of currentTargets) { currentTarget.addEventListener(type, listener, options); } } addEventListeners(document.querySelectorAll('input[value="content-aを開く/閉じる"]'), 'click', { toggleClassList: document.getElementById('content-a').classList, handleEvent: function handleClick() { this.toggleClassList.toggle('opened'); } }, false); </script>
汎用性が高ければ、より広い範囲のコードに対応でき、専用性が高ければ、固有の機能に特化してコードが書きやすくなります。
求めるバランスは人それぞれですので、自分に合ったバランスを模索してください。
Re: bag_ai さん
投稿2021/10/02 03:32
編集2021/10/02 03:53総合スコア18189
0
こんなんはどないです?
menu_open()
関数の引数を以下のように拡張します。
- 第1引数:
toggledItemSelector
--- 第2引数で与えるCSSクラスをトグルしたい要素のセレクタ - 第2引数:
toggledClass
--- 第1引数のセレクタに該当する要素に対してトグルしたいCSSクラス - 残余引数:
...triggerSelectors
--- トグルを行うトリガーとなる(クリッカブルな)要素のセレクタ群
この引数の仕様変更によって、質問にあるmenu_open()
の呼び出し例も、以下のように変わります。
diff
1- menu_open(".header__nav-btn", ".cover-darken", ".js-open"); 2+ menu_open(".js-open", "is-opened", ".header__nav-btn", ".cover-darken");
以下は、この仕様の実装例です。
javascript
1function menu_open(toggledItemSelector, toggledClass, ...triggerSelectors) { 2 // CSSクラスをトグルする対象要素を特定する。(与えられたセレクタで複数要素が該当しても先頭要素のみを対象にする。これは既存を踏襲) 3 const toggledItem = document.querySelector(toggledItemSelector); 4 if (!toggledItem) return; 5 6 // CSSクラスをトグルする関数 7 const handleToggle = () => { 8 toggledItem.classList.toggle(toggledClass); 9 } 10 11 // トグル実行のトリガーとなる要素に、トグルを実行する関数をaddEventListenerする。 12 triggerSelectors.forEach(selector => { 13 [...document.querySelectorAll(selector)].forEach(triggerElm => { 14 triggerElm.addEventListener('click', handleToggle); 15 }) 16 }); 17}
上記のサンプルでも確認できるように、
- トグルを実行する対象を、残余引数で受け取ることで、何個でもセレクタを指定できます。
また、
- トリガー指定のあるセレクタに、複数の要素が該当した場合(たとえば上記のサンプルでは、
.B
ボタン は二つあります。)、そのすべてに、クリックハンドラとして、トグル実行の関数をクリックリスナーとして設定します。
補足:
上記の修正だと、トグルするCSSクラス名("is-open"
)も引数で与えるため、関数名のmenu_open()
も変える必要がありそうです。たとえば
- addClassToggleOnTriggers()
のような名前になるかと思います。
別案1
jQueryもどきを自作してみる、っていうのはどうでしょう? たとえば
javascript
1(function () { 2 3 function myQuery(selector) { 4 const nodeList = document.querySelectorAll(selector); 5 return new myQuery.__clazz__(nodeList); 6 } 7 8 myQuery.__clazz__ = class { 9 10 constructor(nodeList) { 11 this.nodeList = nodeList; 12 } 13 14 on(eventType, handler) { 15 this.nodeList.forEach(node => { 16 node.addEventListener(eventType, handler); 17 }); 18 } 19 20 toggleClass(cssClass) { 21 this.nodeList.forEach(node => { 22 if (node.classList) { 23 node.classList.toggle(cssClass); 24 } 25 }); 26 } 27 } 28 29 window.$ = myQuery; 30})(); 31
ってなものを作っておいて、質問にある例に適用するならば、
javascript
1$('.header__nav-btn, .cover-darken').on('click', function() { 2 $('.js-open').toggleClass('is-opened'); 3});
とすると。
先のサンプルに対して、このjQueryもどきを使うと、こんなん。➡ サンプル2
ですが、こんな怪しげなまがいものを自作して、プチJohn Resigになった自己満足に浸るヒマがあったら、本物を使えるようになったほうが断然いいです。ということで、別案を提示するつもりが、質問の本題からはそれて、「jQueryを覚えてみては?」というのが結論になってしまいました。申し訳ありません。
別案2
この回答のはじめに挙げた
menu_open()
関数の引数を以下のように拡張します。・第1引数:
toggledItemSelector
--- 第2引数で与えるCSSクラスをトグルしたい要素のセレクタ
・第2引数:toggledClass
--- 第1引数のセレクタに該当する要素に対してトグルしたいCSSクラス
・残余引数:...triggerSelectors
--- トグルを行うトリガーとなる(クリッカブルな)要素のセレクタ群
という提案を修正した案を挙げておきます。どこを修正するかというと、
・残余引数:
...triggerSelectors
--- トグルを行うトリガーとなる(クリッカブルな)要素のセレクタ群
の部分です。ここを残余引数で渡すのではなく、ひとつのセレクタの文字列で渡すことができます。というのは、セレクタの文法で、複数のセレクタのOR結合は、カンマ(,
)で区切れば実現できるからです。
また先に書いたように、関数名を addClassToggleOnTriggers
に修正します。以下はこの修正によるコードです。
javascript
1function addClassToggleOnTriggers(toggledItemSelector, toggledClass, triggerSelector) { 2 // CSSクラスをトグルする対象要素を特定する。(与えられたセレクタで複数要素が該当しても先頭要素のみを対象にする。これは既存を踏襲) 3 const toggledItem = document.querySelector(toggledItemSelector); 4 if (!toggledItem) return; 5 6 // CSSクラスをトグルする関数 7 const handleToggle = () => { 8 toggledItem.classList.toggle(toggledClass); 9 } 10 11 // トグル実行のトリガーとなる要素に、トグルを実行する関数をaddEventListenerする。 12 document.querySelectorAll(triggerSelector).forEach(triggerElm => { 13 triggerElm.addEventListener('click', handleToggle); 14 }); 15}
質問にある例に適用するには
javascript
1addClassToggleOnTriggers('.js-open', 'is-opened', '.header__nav-btn, .cover-darken');
とします。➡ サンプル3
コメントに対する回答
コメントへ回答するために、以下のサンプルで説明します。(下記のHTMLを丸ごとコピペしてHTMLファイルを作り、ブラウザで表示させてご確認ください)
html
1<!DOCTYPE html> 2<html lang="ja"> 3<head> 4 <meta charset="UTF-8"> 5 <title>362305@teratail</title> 6 <style> 7 .root { width: 510px; } 8 body > .container { padding-left: 10px; } 9 body > .container > .container { padding-left: 30px; } 10 body > .container > .container > .container { padding-left: 50px; } 11 body > .container > .container > .container > .container { padding-left: 70px; } 12 13 .header__nav-btn, .cover-darken { margin-bottom: 15px; height: 36px; display: flex; align-items: center; padding-left: 5px; font-size: 16pt; cursor: pointer; color: white; } 14 .header__nav-btn { background-color: red; } 15 .cover-darken { background-color: blue; } 16 17 .result { 18 display: flex; 19 align-items: center; 20 justify-content: center; 21 width: 520px; 22 height: 60px; 23 text-align: center; 24 font-size: 20pt; 25 background-color: yellow; 26 } 27 28 .js-open:after { 29 content: 'is-opened: No' 30 } 31 32 .js-open.is-opened:after { 33 content: 'is-opened: Yes' 34 } 35 36 </style> 37 <script> 38 document.addEventListener('DOMContentLoaded', function () { 39 40 function addClassToggleOnTriggers(toggledItemSelector, toggledClass, triggerSelector) { 41 42 // CSSクラスをトグルする対象要素を特定する。(与えられたセレクタで複数要素が該当しても先頭要素のみを対象にする。これは既存を踏襲) 43 const toggledItem = document.querySelector(toggledItemSelector); 44 if (!toggledItem) return; 45 46 // CSSクラスをトグルする関数 47 const handleToggle = () => { 48 toggledItem.classList.toggle(toggledClass); 49 } 50 51 // トグル実行のトリガーとなる要素に、トグルを実行する関数をaddEventListenerする。 52 document.querySelectorAll(triggerSelector).forEach(triggerElm => { 53 console.log(triggerElm.textContent); 54 triggerElm.addEventListener('click', handleToggle); 55 }); 56 } 57 58 addClassToggleOnTriggers('.js-open', 'is-opened', '.header__nav-btn, .cover-darken'); 59 }) 60 </script> 61</head> 62<body> 63 64<div class="container root"> 65 <div class="cover-darken">1</div> 66 <div class="header__nav-btn">2</div> 67 <div class="container"> 68 <div class="header__nav-btn">4</div> 69 <div class="header__nav-btn">5</div> 70 <div class="container"> 71 <div class="cover-darken">9</div> 72 <div class="header__nav-btn">10</div> 73 <div class="container"> 74 <div class="cover-darken">12</div> 75 </div> 76 </div> 77 <div class="cover-darken">6</div> 78 </div> 79 <div class="container"> 80 <div class="container"> 81 <div class="cover-darken">11</div> 82 </div> 83 <div class="header__nav-btn">7</div> 84 <div class="header__nav-btn">8</div> 85 </div> 86 <div class="cover-darken">3</div> 87</div> 88 89<div class="result"> 90 <div class="js-open is-opened"></div> 91</div> 92 93</body> 94</html> 95 96
上記のHTMLをブラウザで表示させると以下のようになります。
- 赤い矩形は
.header__nav-btn
です。青い矩形は.cover-darken
です。 - 赤または青の矩形をクリックすると、黄色の矩形の中のテキストが、is-opened: Yes と is-opened: No とを切り替えます。
- 赤と青の矩形の左端に、白い数字が書かれています。数字の規則は以下です。
・ DOMツリーの中で深い場所にあるものほど、数字は大きくなります。
・ 同じ深さにある矩形は、連続した数字がふられます。
Javascriptの中に
javascript
1// トグル実行のトリガーとなる要素に、トグルを実行する関数をaddEventListenerする。 2document.querySelectorAll(triggerSelector).forEach(triggerElm => { 3 console.log(triggerElm.textContent); 4 triggerElm.addEventListener('click', handleToggle); 5});
というコードがありますが、これの triggerSelector
に渡されるセレクターの文字列は
'.header__nav-btn, .cover-darken'
です。これは .header__nav-btn
と .cover-darken
をカンマ区切りで並べたセレクタリストです。
また、forEach
に渡している関数の中で
javascript
1console.log(triggerElm.textContent);
で、triggerElm
のテキスト内容を出力させています。以下はこれによって出力されたログです。
これは、querySelectorAll('.header__nav-btn, .cover-darken')
で取得される要素のリストに、どのような順番で要素が並んでいるかを表しています。
この取り出される順番について、querySelectorAllの仕様の確認をすると、W3CのSelectors API のドキュメントに以下の記述があります。
6.2. Finding Elements
・・・
The querySelectorAll() methods on the Document, DocumentFragment, and Element interfaces must return a NodeList containing all of the matching Element nodes within the subtrees of the context node, in document order. If there are no matching nodes, the method must return an empty NodeList.
とあり、この中の "in document order" のリンク先に、以下の一文があります。
The term document order means a depth-first pre-order traversal of the DOM tree or subtree in question.
この中にある、 depth-first で pre-order なDOMツリーの巡回をした結果が、先ほどの以下です。
depth-first で pre-order なツリーの巡回というのは、絵で説明すると、以下のような順序でノードをたどる辿り方です。
なお、depth-first (深さ優先)に対照的な辿り方は、breadth-first (幅優先)で、pre-order に対照される辿り方には in-order や post-order があります。ご興味あれば調べてみるとよいかもしれません。
投稿2021/10/01 15:44
編集2021/10/06 17:21退会済みユーザー
総合スコア0
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
退会済みユーザー
2021/10/06 17:32
退会済みユーザー
2021/10/06 17:40
0
ベストアンサー
スマート=短い、ということで無くて良いのであれば以下のようなコードはいかがでしょうか?
function toggleOpen(targetClass) { const target = document.querySelector(targetClass) if (target) target.classList.toggle("is-opened"); } // 第1引数:切り替えの対象のクラス名 , 第2引数以降:切り替えを行うボタンのクラス名(0個~) function assignEvent(targetClass, ...buttonClasses) { buttonClasses .map(buttonClass => { return document.querySelector(buttonClass) }) .forEach(button => { if (button) button.onclick = () => toggleOpen(targetClass) }) }
切り替えボタンのクラスが増減しても、関数呼び出しの引数を調整するだけで対応できます。
assignEvent(".js-open", ".header__nav-btn"); //減らす assignEvent(".js-open", ".header__nav-btn", ".cover-darken", '.hoge', '.fuga', /*or more*/); //増やす
また、同じクラス名のボタンが複数ある場合に対応したければ以下の修正で事足ります。
function assignEvent_all(targetClass, ...buttonClasses) { buttonClasses .map(buttonClass => [...document.querySelectorAll(buttonClass)]) .flat() .forEach(button => { if (button) button.onclick = () => toggleOpen(targetClass) }) }
コメントに対する返答
①document.querySelectorAllで配列で各要素取得
[header__nav-btn, header__nav-btn, header__nav-btn]
querySelectorAll
で取得されるのはNodeList
で配列と似た性質を持ちますが配列とは異なります。
詳しくはMDNなどを参照して下さい。
②...スプレッド構文で展開
③配列リテラルで配列化
②と③はセットの処理です。
NodeList
→Array
へ変換するために一度NodeListをNodeに展開しています。NodeListのままではmap
やflat
等のArrayインスタンスメソッド全般が使用できません。
Array.from
等を使用しても同等です。
④その後cover-darkenクラスがあれば合算していく
[header__nav-btn , header__nav-btn , header__nav-btn , cover-darken]
合算という言葉は少し正しくないような気がします(認識は同じかもしれません)
[...document.querySelectorAll(".header__nav-btn")]
→[headerBtn0,headerBtn1]
[...document.querySelectorAll(".cover-darken")]
→[coverDarken0,coverDarken1]
上記のように取得要素が取得された場合、map
メソッドでは2次元の配列([[headerBtn0,headerBtn1],[coverDarken0,coverDarken1]]
)が作られます
この配列に対してflat
を書けることで1次元の配列[headerBtn0,headerBtn1,coverDarken0,coverDarken1]
となり、取得に使用したクラス関係なく一括でforEach
にかけられるので便利だよね、という感じです。
追記
今試してみたらquerySelectorAll
は普通に取得条件を配列のまま渡しても要素が取得されました。
なのでArrayメソッドでこねくり回さなくても以下の単純なコードで良さそうです。前の回答はArrayメソッドの参考程度に考えて下さい。
function assignEvent_all(targetClass, ...buttonClasses) { document.querySelectorAll(buttonClasses) .forEach(button => { if (button) button.onclick = () => toggleOpen(targetClass) }) }
投稿2021/10/01 14:47
編集2021/10/04 07:45総合スコア983
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2021/10/04 01:33
2021/10/04 09:42
あなたの回答
tips
太字
斜体
打ち消し線
見出し
引用テキストの挿入
コードの挿入
リンクの挿入
リストの挿入
番号リストの挿入
表の挿入
水平線の挿入
プレビュー
質問の解決につながる回答をしましょう。 サンプルコードなど、より具体的な説明があると質問者の理解の助けになります。 また、読む側のことを考えた、分かりやすい文章を心がけましょう。
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2021/10/06 12:44
2021/10/07 03:08