投稿者: 
現在、もりけん塾にてJavaScriptを勉強させていただいております。その記録。
今回は課題19についての備忘録です。
考え方が間違っている箇所もあるかと思いますが、お気づきの際はご指摘いただければと思います。
課題19について
仕様
スライドショーコンテンツUI（課題16）とスライドショー（課題17、18）を併せたページを作ってください。
制作物
コードはこちらから(codesandbox)※codesandboxではエラーが出る場合がありますが、リロードすれば動きます。
実装内容
課題16（ニュースUIコンポーネント）から変わった点
①記事投稿が3日以内だったらnewアイコン表示(date-fns使用)
前回課題16で「記事投稿が3日以内だったらnewのアイコンをつける」という仕様の際、単純な計算で実装していたのを今回date-fnsというパッケージを使用して実装してみました。
date-fnsを使用するためにVite環境を用意し、ローカルで確認を行いました。（ここが一番大変だった・・）
Viteの記事についてはこちらの記事（【もりけん塾】Viteハンズオンをやってみた）を。
今回、differenceInDaysというメソッドを使用しました。こちらは、指定した２つの日付についての日数の差を出してくれるメソッドとなっているので、今回にピッタリと使用しました。
ここで出た日数の差分を使用して、3日以下であればnewアイコンが表示するようになります。
前回は計算で出していたのですが、こういうパッケージを使用した方が断然楽だなと思いました。このパッケージを使用するという環境を用意するのがイメージ湧かなくて尻込みしてましたが、今回挑戦できてよかった。
import { differenceInDays } from "date-fns";
// Get the number of days elapsed
const getDifferenceDays = (postDateData) => {
return differenceInDays(new Date(), new Date(postDateData));
};
//定数名を変更しました
//const getElapsedDays = (postDateData) => {
// const elapsedDays = differenceInDays(new Date(), new Date(postDateData));
// return elapsedDays;
//};ここについては、定数名について、elapsedDaysを使用していましたが、メソッドに適応させるとより理解しやすいとレビューいただき、getDifferenceDaysに変更しました。
②for文をfor ofに書き換え
至る所で、for文やfor each、for ofが出てきていたので、一旦for ofにまとめられないかとトライしてみました。が、結果的にそこまでfor ofにこだわる必要はないということがわかりました。
例えば、下記のタブの切り替え部についてfor ofに書き換えてみたのですが、このコードでは各liに一つずつイベント関数を与えているので、パフォーマンスがあまり良くないとレビューいただきました。
//Tab switching function
const clickedTabs = () => {
tabsGroup.addEventListener("click", (e) => {
const activeTab = document.getElementsByClassName("is-active-tab")[0];
const activeContent = document.getElementsByClassName("is-active-content")[0];
activeTab.classList.remove("is-active-tab");
e.target.classList.add("is-active-tab");
activeContent.classList.remove("is-active-content");
const contents = document.getElementsByClassName("contentsContainer");
contents[e.target.dataset.index].classList.add("is-active-content");
});
};
//却下したコード
// const clickedTabs = () => {
// const tabLists = document.getElementsByClassName("tabList");
// const contents = document.getElementsByClassName("contentsContainer");
// for (const tabList of tabLists) {
// tabList.addEventListener("click", function () {
// const arrayTabsLists = Array.from(tabLists);
// const index = arrayTabsLists.indexOf(this);
// document.querySelector(".is-active-tab").classList.remove("is-active-tab");
// arrayTabsLists[index].classList.add("is-active-tab");
// document.querySelector(".is-active-content").classList.remove("is-active-content");
// contents[index].classList.add("is-active-content");
// });
// }
// };また、indexを取得するのであれば、for文かfor each を使用した方がより簡単にindexを取得できるのではないかということもレビューいただきました。元々ここについてはfor文でindexを取得していたので、まさにその通りでした。ここではfor文使用が適切だと判断。元に戻しました。
//get Tab Elements
const getTabListsFragment = (newsData) => {
const fragmentTablists = document.createDocumentFragment();
for (let i = 0; i < newsData.length; i++) {
const tabList = createElementWithClassName("li", "tabList");
tabList.textContent = newsData[i].category;
tabList.dataset.index = i;
newsData[i].initialDisplay && tabList.classList.add("is-active-tab");
fragmentTablists.appendChild(tabList);
}
return fragmentTablists;
};
//for ofにした時
//const getTabListsFragment = (newsData) => {
// const fragmentTablists = document.createDocumentFragment();
// for (const topics of newsData) {
// const tabList = createElementWithClassName("li", "tabList");
// tabList.textContent = topics.category;
// topics.initialDisplay && tabList.classList.add("is-active-tab");
// fragmentTablists.appendChild(tabList);
// }
// return fragmentTablists;
//};単純に、for ofで書くとスッキリする気がしたのですが、使いたい値ややりたい仕様によって適切なループ処理を選択するのが良いのだと勉強になりました。考えが浅はかだったなと反省。
課題17,18（スライドショー実装）から変更した点
大きく変えたのは、アローボタンに対する関数を機能によって分けました。
元々はボタンを作成するのとクリックイベントを同じ関数に入れていたのですが、ここは分けた方がいいのではとレビューいただきました。言われてみるとそうだなと。当時はそのアイディアが湧かなかったし、できるかな？と思ったのですがトライしたらできました。やったー。
まずは、ボタンに対して共通のクラスを付与してあげる。また、それぞれのボタンに対して、どのボタンか判断するためのvalue(prev/next)を付与する。
そのクラスに対してquerySelectorで要素を取得してきて、ボタンが押された際に、そのvalueの値を判断して、それぞれの処理(imgNum.countの増減)を実行させる。
分けた方が読みやすいコードだと思いました。
const createArrowBtnsElements = () => {
const btnDirections = ["next", "prev"];
btnDirections.forEach((btnDirections) => {
const btn = document.createElement("button");
btn.classList.add(`${btnDirections}`, "arrowbutton");
btn.id = btnDirections === "prev" ? "js-prevbtn" : "js-nextbtn";
btn.textContent = btnDirections === "prev" ? "◀︎" : "▶︎";
btn.value = btnDirections;
imgListsWrapper.appendChild(btn);
btn.disabled = btnDirections === "prev";
});
};
const arrowBtnsClickEvent = (imgData) => {
const arrowButtons = document.querySelectorAll(".arrowbutton");
arrowButtons.forEach((button) => {
button.addEventListener("click", (e) => {
e.currentTarget.value === "prev" ? (imgNum.count -= 1) : (imgNum.count += 1);
document.querySelector(".is-show").classList.remove("is-show");
imgList[imgNum.count].classList.add("is-show");
switchPagenation(imgNum.count);
switchDisableForBtn(imgData);
setNumberOfPage(imgData);
resetAutoPlay(imgData);
});
});
};
//元のコード
// const createBtnElements = (imgData) => {
// const btnDirections = ["next", "prev"];
// btnDirections.forEach((btnDirections) => {
// const btn = document.createElement("button");
// btn.classList.add(`${btnDirections}`);
// btn.id = btnDirections === "prev" ? "js-prevbtn" : "js-nextbtn";
// btn.textContent = btnDirections === "prev" ? "◀︎" : "▶︎";
// imgListsWrapper.appendChild(btn);
// btn.disabled = btnDirections === "prev";
// btn.addEventListener("click", function () {
// btnDirections === "prev" ? (imgNum.count -= 1) : (imgNum.count += 1);
// document.querySelector(".is-show").classList.remove("is-show");
// imgList[imgNum.count].classList.add("is-show");
// switchPagenation(imgNum.count);
// switchDisableForBtn(imgData);
// setNumberOfPage(imgData);
// resetAutoPlay(imgData);
// });
// });
// };ボタンのdisabled切り替え箇所について
ボタンのdisabledの切り替えについて、imgNumが0とラストの時をそのまま挿入していましたが、ここの数字を変数に入れることで、どんな状況の際に切り替わるのか理解しやすいのではとレビューいただきました。本当にその通りと思ったので、そのまま変更しました。自分だけでやっていると気づかないことが多いです。
const switchDisableForBtn = (imgData) => {
const prevBtnElement = document.getElementById("js-prevbtn");
const nxtBtnElement = document.getElementById("js-nextbtn");
const firstNum = 0;
const lastNum = imgData.length - 1;
prevBtnElement.disabled = imgNum.count === firstNum;
nxtBtnElement.disabled = imgNum.count === lastNum;
//元々は直接入れていた
// prevBtnElement.disabled = imgNum.count === 0;
// nxtBtnElement.disabled = imgNum.count === lengthImg - 1;
};その他レビューPOINT
appendChildの記載ミス
appendChildをappendと記載している箇所がありました。単純なミスだったのですが、動きは変わらないのかと気になり調べてみました。そもそもappendは今まで一度も使用したことがなかった。
調べた結果、基本的に動きは同じ模様。appendの特徴は下記。
- IE、Edgeで動作しない
- DOMだけではなくテキストノードも追加できる
- 引数を複数渡せる
- 戻り値がない
正直あまり馴染みがないのだけど、使う場面が思い浮かばない。
取り急ぎ、appendChildに修正しました。
type=”module”にdeferは不要
最初のPR時、type=”module”とdeferを共存させてしまっていたのですが、type=”module”の際は自然とdeferと同じ動きとなるようです。なのでdeferは削除しました。
これは言われないとわからなかったし、調べるきっかけになったので、勉強になりました。
また、type=”module”の際は強制的にstrictモードとなることがわかりました。
<script type="module" src="./js/newsTopics.js"></script>
<script type="module" src="./js/slideshow.js"></script>後記
一応今までapproveいただいた課題なので、そこまで指摘事項はないと思っていたのですが沢山あった。
というのも、パッケージを使用してみたり、少し書き換えてみたりしたので指摘があって当たり前なのですが、少し見落としも多かったなという印象。お手数おかけしすみません。。
とにもかくにも、皆様のレビューによってまたもやコードは綺麗になり、大変勉強になりました。
c.sakyou