Javascript loop of if statement

TLDR:

// with jQuery
var allPages = $('[id^=bfPage]')
var activeId = allPages.filter(':visible').attr('id')

// with ES6
const allPages = [].slice.apply(document.querySelectorAll('[id^=bfPage]'));
let activeId = allPages.find( e => e.style.display !== "none").id;

First of all you do not need to nest your ifs using else statements, because you are using return statement inside each of them. So after first refactoring you are going to have:

function()
{
  if(document.getElementById('bfPage1').style.display !== "none")
   {return document.getElementById('bfPage1').getAttribute('id');
  }

  if(document.getElementById('bfPage2').style.display !== "none")
    {return document.getElementById('bfPage2').getAttribute('id');
    }

  if(document.getElementById('bfPage3').style.display !== "none")
       {return document.getElementById('bfPage3').getAttribute('id');
       }
  if(document.getElementById('bfPage4').style.display !== "none")
       {return document.getElementById('bfPage4').getAttribute('id')
       }
}

Next step is to notice that that each time you are quering element twice:

function()
{
  var page
  page = document.getElementById('bfPage1')
  if(page.style.display !== "none")
  {
    return page.getAttribute('id');
  }
  page = document.getElementById('bfPage2')
  if(page.style.display !== "none")
  {
    return page.getAttribute('id');
  }
  page = document.getElementById('bfPage3')
  if(page.style.display !== "none")
  {
    return page.getAttribute('id');
  }
  page = document.getElementById('bfPage4')
  if(page.style.display !== "none")
  {
    return page.getAttribute('id')
  }
}

Now you clearly see that you can use loop instead of repeating statements

function()
{
  var pages = ['bfPage1','bfPage2','bfPage3','bfPage4']
  for (var i in pages) {
    page = document.getElementById(pages[i])
    if(page.style.display !== "none")
    {
      return page.getAttribute('id');
    }
  }
}    

Oh and you already know id

function()
{
  var pages = ['bfPage1','bfPage2','bfPage3','bfPage4']
  for (var i in pages) {
    page = document.getElementById(pages[i])
    if(page.style.display !== "none")
    {
      return pages[i];
    }
  }
} 

Which is quite nice.

You can use jquery and make one-liner out of it:

$('[id^=bgPage]:visible]').attr('id') 

This is good result but lets think why do we need to do this in the first place?

You have some logic to show and hide elements of the page and you want to check which one is visible.

Instead of doing that with styles of elements, lets do two things.

• add new class for every element .bfPage

• lets add class visible or active to the only visible element

This way your code to get id will be changed to:

$('.bfPage.active').attr('id')

Which in case of real application will be split on two parts:

somewhere in the beginning you will have

var allPages = $('.bfPages')

and somewhere where you need to find that element

var activeId =  allPages.filter('.active').attr('id')

Of course if it is not the case you still can improve performance caching all bfPages

var allPages = $('[id^=bfPage]')

...

var activeId = allPages.filter(':visible').attr('id')