0

I wrote this simple carousel but without encapsulation. So previously I placed items from buttonControl() in global scope and added eventListeners on global scope, which are now emraced in prev() and next() functions. However, my encapsulation breaks the code. Because arguments from buttonControl() aren't global but prev() and next() needs them to work. I thought that maybe I can pass all arguments from buttonsControl() inside addEventListener('click', prev) but I cannot, because when I write thisaddEventListener('click', prev(slides,totalItems,allItems....)) it is launching this event without a click.And I even don't know if its correct way.

I thought of puting arguments from buttonsControl() inside prev() and next() but it won't work.


function buttonsControl(){
const slides = document.querySelector('.slides');
const totalItems = document.querySelectorAll('.slides>*').length - 1;
const allItems = document.querySelectorAll('.slides>*').length;
console.log(totalItems)
let activeItem = 0;

let controlCarouselFooter = document.querySelector('.carousel_footer');
controlCarouselFooter.innerHTML = `1 / ${allItems}`
console.log(controlCarouselFooter)
const prevButton = document.querySelector('.prev_button').addEventListener('click', prev)
const nextButton = document.querySelector('.next_button').addEventListener('click', next)
// no idea how to pass those arguments
}

// Buttons controls

function prev(){
// const prevButton = document.querySelector('.prev_button').addEventListener('click', () => {
 
if (activeItem === 0) {
    activeItem = totalItems;
    slides.style.transform = `translateX(-${totalItems * 100}%)`;
    console.log(`if ${activeItem}`)
    controlCarouselFooter.innerHTML = `${activeItem+1} / ${allItems}`
  }else {
    activeItem--;
    slides.style.transform = `translateX(-${activeItem * 100}%)`;
    console.log(`else ${activeItem}`)
    controlCarouselFooter.innerHTML = `${activeItem+1} / ${allItems} `
  }
  }
//   );
// }

function next(){
  // const nextButton = document.querySelector('.next_button').addEventListener('click', () => {
 
  if(activeItem < totalItems) {
    activeItem++;
      slides.style.transform = `translateX(-${activeItem * 100}%)`;
      console.log(`if ${activeItem}`)
      controlCarouselFooter.innerHTML = `${activeItem+1} / ${allItems}`
  } else {
    activeItem = 0;
    slides.style.transform = 'none';
    console.log(`else ${activeItem+1}`)
    console.log(`totalItems ${totalItems}`)
    controlCarouselFooter.innerHTML = `${activeItem+1} / ${allItems}`
  }
}
// );
// };
// });
buttonsControl();

Chumba
  • 1
  • 1
  • I agree with the previous statement a bit... what is the error you're getting exactly? I think I interpret your question to mean that the variables such as `allItems` or `activeItems` are throwing an exception becuase they are no longer global, so the function scope doesnt have access to the variables anymore. – Ja Superior Feb 05 '23 at 20:10
  • Yes I am getting the error that activeItems is not defined. And I assumes that everything from buttonsControl() is not defined in next() and prev(). I dont know how to pass those arguments. I thought I can do addEventListener('click', prev(allItems,activeItems etc etc) but then this event invoke the function without clicking. – Chumba Feb 05 '23 at 20:15

2 Answers2

1

The easiest solution would be to define the functions prev and next inside the buttonsControl function, so that all its local variables are in scope through closure:

function buttonsControl() {
  const slides = document.querySelector('.slides');
  const totalItems = document.querySelectorAll('.slides>*').length - 1;
  const allItems = document.querySelectorAll('.slides>*').length;

  let activeItem = 0;
  let controlCarouselFooter = document.querySelector('.carousel_footer');

  controlCarouselFooter.innerHTML = `1 / ${allItems}`;

  const prevButton = document.querySelector('.prev_button').addEventListener('click', prev);
  const nextButton = document.querySelector('.next_button').addEventListener('click', next);
    
  // Buttons controls
  function prev() {
    if (activeItem === 0) {
      activeItem = totalItems;
    } else {
      activeItem--;
    }
    slides.style.transform = `translateX(-${totalItems * 100}%)`;
    controlCarouselFooter.innerHTML = `${activeItem+1} / ${allItems}`
  } 
  function next() {
    if (activeItem < totalItems) {
      activeItem++;
      slides.style.transform = `translateX(-${activeItem * 100}%)`;
    } else {
      activeItem = 0;
      slides.style.transform = 'none';
    }
    controlCarouselFooter.innerHTML = `${activeItem+1} / ${allItems}`
  }
}
buttonsControl();
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
0

If I'm understanding your question correctly, You could bind the variables to the listeners.

EDIT: someone pointed out that you're mutating activeItems, which is true. So you will want to define all your variables on an object first so that mutation is persistent between function calls.

function buttonsControl() {
    let obj = {};
    obj.slides = document.querySelector('.slides');
    obj.totalItems = document.querySelectorAll('.slides>*').length - 1;
    obj.allItems = document.querySelectorAll('.slides>*').length;

    console.log(obj.totalItems)

    obj.activeItem = 0;

    obj.controlCarouselFooter = document.querySelector('.carousel_footer');
    controlCarouselFooter.innerHTML = `1 / ${allItems}`
    console.log(controlCarouselFooter)

    // bind the variables you want to use to your input
    const prevButton = document.querySelector('.prev_button').addEventListener('click',  prev.bind(null, obj))
    const nextButton = document.querySelector('.next_button').addEventListener('click', next.bind(null, obj))
    // no idea how to pass those arguments
}

// Buttons controls

function prev(obj) {
    //define the arguments in your fn params. 
    // const prevButton = document.querySelector('.prev_button').addEventListener('click', () => {
 
    if (obj.activeItem === 0) {
        obj.activeItem = totalItems;
        obj.slides.style.transform = `translateX(-${totalItems * 100}%)`;
        console.log(`if ${activeItem}`)
        obj.controlCarouselFooter.innerHTML = `${obj.activeItem+1} / ${obj.allItems}`
    } else {
        obj.activeItem--;
        obj.slides.style.transform = `translateX(-${activeItem * 100}%)`;
        console.log(`else ${obj.activeItem}`)
        obj.controlCarouselFooter.innerHTML = `${obj.activeItem+1} / ${obj.allItems} `
    }
  }
//   );
// }

function next(obj) {
    // ...similar implementation to prev()
}
// );
// };
// });

buttonsControl();
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Ja Superior
  • 459
  • 4
  • 17
  • In general, yes, but it doesn't work here since you cannot mutate `activeItem`. – Bergi Feb 05 '23 at 20:20
  • Ah! you're right... I didnt even pay attention to that little line of code. Your solution provides to better answer. I suppose an edit to my solution would be to define all the significant variables on an object thats shared between the two functions, so that they can be mutated. – Ja Superior Feb 05 '23 at 20:22
  • 1
    Yeah, that would work, basically making `prev` and `next` methods - then use `class` syntax… I thought about it but didn't want to completely refactor everything :-) – Bergi Feb 05 '23 at 20:24
  • it would be better to define it in a class. But I provided an editted solution that doesnt require a full refactor. ;-). Still, I think your solutions requires the lowest burden on the amount of code he'd need to rewrite. – Ja Superior Feb 05 '23 at 20:28