Vanilla JS でclassを使ってコードを整理を行い再利用したい

Question

JS初学者です。Vanilla JS でスクロールフェードインアニメーションを実装しました、よくあるスクロールしていったら指定しているclassのエレメントにaddClassして、フェードインさせるやつです。

以下のコードでscrollinというclassを付与してフェードインできます
///// スクロールでフェードイン /////
const els = document.querySelectorAll('.fade-in');

els.forEach(function(fadeIn) {
const windowHeight = window.innerHeight;

window.addEventListener('scroll', function() {
const offset = fadeIn.getBoundingClientRect().top;
const scroll = window.scrollY;
if(scroll > offset - windowHeight + windowHeight/15){
fadeIn.classList.add('scrollin');
}
})
})
上記のコードをclass(シンタックスシュガー)を使用して、対象のエレメントと付与するclassを変えたいと思っているのですが、なかなかうまくいきません。
どなたかご教示いただけないでしょうか。
※コードをなるべく冗長にしたく無いので、classを使用した記述にトライしたのですが、そもそもclassでは無い記述が良かったりしますでしょうか？

以下は作成したコードです。

class fadeIn {
    constructor(el, In){
        this.el = document.querySelector(el);
        this.In = In;
    }
    anime(){

        this.el.forEach(function(fade) {
            const windowHeight = window.innerHeight;
            
            window.addEventListener('scroll', function() {
              const offset = fade.getBoundingClientRect().top;
              const scroll = window.scrollY;
              
              if(scroll > offset - windowHeight + windowHeight/15){
                 fade.classList.add(this.In);
              }
            })
          })
      
    }
}
const FadeAnimation = new fadeIn('.fade-in2-1','.scroll2');
const FadeAnimation2 = new fadeIn('.fade-in2-2','.scroll3');

FadeAnimation.anime();
FadeAnimation2.anime();

試したこと

以下のサイトを参考に色々試してみましたが、うまくいきません。。。
https://qiita.com/shotets/items/e4b2ca36f5cbdc393117
https://techplay.jp/column/482

Answer 1

「うまくいきません。。。」が具体的にどういう状況なのかわかりませんが、とりあえず、fadeIn() コンストラクタの第2引数の先頭の . は不要に見えます。

引数の名前はわかりやすくしましょう。私なら、el は targetSelector、In は classNameToBeAdded にします。

Comments

[nakanakasi]

int32_t様
ご指摘ありがとうございます、引数の名前は取り急ぎ変更しました。

それと具体的に記載できておらずすいませんでした、エラーを調べたところ以下のような表記が出ていました。
script.js:110 Uncaught TypeError: this.targetSelector.forEach is not a function
at fadeIn.anime (script.js:110:29)
at HTMLDocument.<anonymous> (script.js:93:17)

this.targetSelector.forEachの箇所が関数ではありませんとエラー表示されています(引数は書き換えています)。
記述が何か違うのでしょうか？
もし対応方法ご存知でしたらご教示いただけますと幸いです。

何卒宜しくお願いいたします。

[int32_t]

ああ、this.el は配列ではないですから、forEach は使えません。「this.el.forEach(function(fade) {」を削除、fade を this.el に置き換え、でどうでしょうか。

this.targetSelector は名前としては不適切です。this.targetElement などでしょう。targetSelector という名前は引数についてのみです。

[nakanakasi]

int32_t様
ご返答ありがとうございます。

以下のような記述であっていますでしょうか？
※引数は一旦戻しています

class fadeIn {
constructor(el, In){
this.el = document.querySelector(el);
this.In = In;
}
anime(){

const windowHeight = window.innerHeight;

window.addEventListener('scroll', function() {
const offset = this.el.getBoundingClientRect().top;
const scroll = window.scrollY;

if(scroll > offset - windowHeight + windowHeight/15){
this.el.classList.add(this.In);
}
})
}

}

上記の場合以下のようなエラーが出ています。
script.js:137 Uncaught TypeError: Cannot read properties of undefined (reading 'getBoundingClientRect')
script.js:137 Uncaught TypeError: 未定義のプロパティを読み取れません (「getBoundingClientRect」を読み取っています)
とのことなのですが、、、初学者の私にはどのような対応したらよいかわからないでいます。。
申し訳ありません、ご教示いただけますと幸いです。

何卒宜しくお願いいたします。

[int32_t]

もう一つ問題がありましたね。
「window.addEventListener('scroll', function() {」を「window.addEventListener('scroll', () => {」に変えてみてください。前者の場合、イベントリスナの関数中では this が window になります。

[nakanakasi]

int32_t様

ご返答ありがとうございます、動作しました。

恐縮ですがもう一点質問です、もしクラス付与したい対象のものが複数あった場合(以下のようなDOMの場合)
<div class="fade-in2-1">1個目</div>
<div class="fade-in2-1">2個目</div>
<div class="fade-in2-1">３個目</div>

配列にしてループで回さないといけないと思うのですが、以下のような記述で良いでしょか？
class fadeIn {
constructor(el, In){
this.el = document.querySelectorAll(el);
this.In = In;

}
anime(){
this.el.forEach(element => {

const windowHeight = window.innerHeight;

window.addEventListener('scroll', () => {
const offset = element.getBoundingClientRect().top;
const scroll = window.scrollY;

if(scroll > offset - windowHeight + windowHeight + 500){
element.classList.add(this.In);

}
})
})
}
}

上記で一応エラー出ずに動作はしました。

お手数ですがご確認いただけますと幸いです、何卒宜しくお願いいたします。

[int32_t]

動くのならそれでいいと思います。
scroll イベントリスナは1つだけ登録してその中でループしたほうがメモリ効率はいいかもしれません。

window.addEventListener('scroll', () => {
this.el.forEach(element => {
const offset = element.getBoundingClientRect().top;

[nakanakasi]

色々とアドバイスありがとうございました。
とれも参考になりました。