3
\$\begingroup\$

I have the following code and I'm trying to find a cleaner way to run the year functions. At the moment I have 5 years as divs with an onclick event to bring the appropriate year image into view. So if you click on 1950, the 1950 year image will appear. If you then click on 1960, the 1950 image will disappear and the 1960 image will appear and so on.

In order to make sure that only the year image I want to see appears I have coded each function to remove every other years' image but this is going to lead to massive bloat as I want to be able to increase my 5 years to 20 and up.

Is there a way such as using a loop to work something like: "If I click on 1950 then hide anything other 1950 and just show 1950" rather than what I have now which is "If I click on 1950, then hide 1960 image, hide 1970 image, hide 1980 image... and show 1950 image"

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<base href="http://dtwdev.co.uk/new/">
<title>Year test</title>


<script>
<!-- Function to bring image onto screen -->
function slideInYear(el){
	var elem = document.getElementById(el);
	elem.style.transition = "left 0.5s linear 0s";
	elem.style.left = "50px";
}

<!-- Function to send image back off screen -->
function slideOutYear(el){
	var elem = document.getElementById(el);
	elem.style.transition = "left 0.5s linear 0s";
	elem.style.left = "-500px";
}

<!-- 1950 function brings 1950 image on screen and makes sure all others are sent off -->
function year1950() {
	slideInYear('year_1950'),
	slideOutYear('year_1960'),
	slideOutYear('year_1970'),
	slideOutYear('year_1980'),
	slideOutYear('year_1990')
}

<!-- 1960 function brings 1960 image on screen and makes sure all others are sent off -->
function year1960() {
	slideInYear('year_1960'),
	slideOutYear('year_1950'),
	slideOutYear('year_1970'),
	slideOutYear('year_1980'),
	slideOutYear('year_1990')
}

function year1970() {
	slideInYear('year_1970'),
	slideOutYear('year_1960'),
	slideOutYear('year_1950'),
	slideOutYear('year_1980'),
	slideOutYear('year_1990')
}

function year1980() {
	slideInYear('year_1980'),
	slideOutYear('year_1960'),
	slideOutYear('year_1970'),
	slideOutYear('year_1950'),
	slideOutYear('year_1990')
}

function year1990() {
	slideInYear('year_1990'),
	slideOutYear('year_1960'),
	slideOutYear('year_1970'),
	slideOutYear('year_1980'),
	slideOutYear('year_1950')
}


</script>

<style>
/*This code makes sure the five images are held off screen until needed */
div#year_1950 {position:absolute;top:0px;left:-500px;}
div#year_1960 {position:absolute;top:0px;left:-500px;}
div#year_1970 {position:absolute;top:0px;left:-500px;}
div#year_1980 {position:absolute;top:0px;left:-500px;}
div#year_1990 {position:absolute;top:0px;left:-500px;}



</style>
</head>

<body>

<!-- These five divs are hidden off the left of the screen -->
<div id="year_1950"><img src="bg_1950.jpg"></div>
<div id="year_1960"><img src="bg_1960.jpg"></div>
<div id="year_1970"><img src="bg_1970.jpg"></div>
<div id="year_1980"><img src="bg_1980.jpg"></div>
<div id="year_1990"><img src="bg_1990.jpg"></div>

<!-- These five divs have the dates that the user clicks to bring the image on screen -->
<div><a href="javascript:void(0);" onclick="year1950()">1950</a></div>
<div><a href="javascript:void(0);" onclick="year1960()">1960</a></div>
<div><a href="javascript:void(0);" onclick="year1970()">1970</a></div>
<div><a href="javascript:void(0);" onclick="year1980()">1980</a></div>
<div><a href="javascript:void(0);" onclick="year1990()">1990</a></div>


<script src="http://www.dtwdev.co.uk/new/js/jquery.min.js"></script>	
<script src='http://www.dtwdev.co.uk/new/js/css3-animate-it.js'></script>
</body>
</html>

\$\endgroup\$

migrated from stackoverflow.com Oct 1 '15 at 13:06

This question came from our site for professional and enthusiast programmers.

1
\$\begingroup\$

Yes, there is definitely a way to do this. Start by giving all your divs a common class, let's say yearDiv. Then each function can have an onClick event linked to this general function:

function displayOnlyThisYear(clicked_id){
    // slide in the selected year
    slideInYear(clicked_id) 

    // find all other years and slide them out
    var allYears = document.getElementsByClassName('yearDiv');
    var i = 0;
    while(allYears[i]){
        if(allYears[i].id !== clicked_id) slideOutYear(allYears[i].id)
    }
}

Then add this function to your boxes with the following tag inside your div element opening tag:

onClick = "displayOnlyThisYear(this.id)"

Alternately if you are open to using jQuery you can look into using toggleClass combined with a special css class, selected for when a particular year has been selected (so setting up all the attributes of the selected box in here) which should get you what you want in even fewer lines of code. That would get you to something like:

$('.yearDiv').click(function(){
    $('.yearDiv').not(this).removeClass('selected');
    $(this).toggleClass('selected');
}

removeClass and addClass also accept a second argument indicating a time duration for an animation so you can add that to suit your transition style.

\$\endgroup\$
  • \$\begingroup\$ Hi Sunny, Thanks for your reply - I prefer to use the javascript methid rather than jquery. Would I add the class to the DIVs with id="year_1950" to "year_1990"? Do I need to change anything else - I'm getting some 'uncaught style errors, null' from Chrome's dev tools \$\endgroup\$ – user2840467 Oct 2 '15 at 12:33
  • \$\begingroup\$ @user2840467 I typed out the code above but did not try it in a browser. Yes, you would add the class to all the DIV's with id's between "year_1950" and "year_1990". Let me know if you still have trouble and I'll check my code. \$\endgroup\$ – sunny Oct 2 '15 at 13:00
  • \$\begingroup\$ Would appreciate that Sunny, I've made those changes to dtwdev.co.uk/new/test.html I'm not sure I've totally understood you. \$\endgroup\$ – user2840467 Oct 3 '15 at 17:57
1
\$\begingroup\$

Create a style for the classes hidden_year and visible_year. When you click a link use a selector that selects all with id containing "year_19" and set them all to hidden_year, then just set the link you clicked to visible_year.

You could use a loop to build up the id values 5-9 if you really wanted to. . . var curr_year = 'year_19' + i + '0' in a loop.

Here is an example:

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Year test</title>

<style>
.hidden_year {
    transition: left 0.5s linear 0s;
    position:absolute;
    top:0px;
    left:-500px;

}

.visible_year {
    transition: left 0.5s linear 0s;
    position:absolute;
    top:0px;
    left:50px;
}

</style>
<script>

function process(id) {
$('[id*="year_19"]').each(function(index) {
        $(this).removeClass().addClass('hidden_year');
    });
$('[id=' + id + ']').removeClass().addClass('visible_year');
}


</script>

</head>

<body>

<div id="year_1950" class="hidden_year"><img src="http://dtwdev.co.uk/new/bg_1950.jpg"></div>
<div id="year_1960" class="hidden_year"><img src="http://dtwdev.co.uk/new/bg_1960.jpg"></div>
<div id="year_1970" class="hidden_year"><img src="http://dtwdev.co.uk/new/bg_1970.jpg"></div>
<div id="year_1980" class="hidden_year"><img src="http://dtwdev.co.uk/new/bg_1980.jpg"></div>
<div id="year_1990" class="hidden_year"><img src="http://dtwdev.co.uk/new/bg_1990.jpg"></div>


<div><a href="javascript:void(0);" onclick="process('year_1950')">1950</a></div>
<div><a href="javascript:void(0);" onclick="process('year_1960')">1960</a></div>
<div><a href="javascript:void(0);" onclick="process('year_1970')">1970</a></div>
<div><a href="javascript:void(0);" onclick="process('year_1980')">1980</a></div>
<div><a href="javascript:void(0);" onclick="process('year_1990')">1990</a></div>





<script src="http://dtwdev.co.uk/new//js/jquery.min.js"></script>   
<script src='http://dtwdev.co.uk/new//js/css3-animate-it.js'></script>
</body>
</html>
\$\endgroup\$
-1
\$\begingroup\$
<div><a href="javascript:void(0);" onclick="slideInYear('year_1950')">1950</a></div>
<div><a href="javascript:void(0);" onclick="slideInYear('year_1960')">1960</a></div>
<div><a href="javascript:void(0);" onclick="slideInYear('year_1970')">1970</a></div>
<div><a href="javascript:void(0);" onclick="slideInYear('year_1980')">1980</a></div>
<div><a href="javascript:void(0);" onclick="slideInYear('year_1990')">1990</a></div>   

 <!-- Function to bring image onto screen -->
    function slideInYear(el){
        var elem = document.getElementById(el);
        elem.style.transition = "left 0.5s linear 0s";
        elem.style.left = "50px";
        slideOutYear();
        elem.className = 'active';
    }

<!-- Function to send image back off screen -->
function slideOutYear(){
    var active = document.getElementsByClassName('active');
    if (active.length > 0) {
      var elem = active[0];
      elem.style.transition = "left 0.5s linear 0s";
      elem.style.left = "-500px";
      elem.className = '';
    }
}

change these things

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and how it improves upon the original) so that the author can learn from your thought process. \$\endgroup\$ – SuperBiasedMan Oct 1 '15 at 20:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.