Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: static/js/scripts.js

Issue 29335113: Issue 2675 - Implemented responsive header for web.eyeo.com (Closed)
Left Patch Set: Minor code style and variable scope cleanup Created Feb. 29, 2016, 5:03 p.m.
Right Patch Set: Fixed code style and consistancy issues Created June 23, 2016, 5:23 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « static/js/respond.min.js ('k') | templates/default.tmpl » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 jQuery(function() 1 (function(){
saroyanm 2016/04/08 16:56:10 Note: I think if we could use Javascript instead o
juliandoucette 2016/04/25 19:08:56 - I agree that we should get rid of jquery - But I
saroyanm 2016/05/06 14:49:28 I think if we will decide to get rid of animation
juliandoucette 2016/05/11 18:11:06 Ok, I will implement this without jQuery.
2 { 2 function addListener(target, event, callback)
3 var $toTop = jQuery("#to-top"); 3 {
saroyanm 2016/04/08 16:56:09 Why to use dollar sign in front of the variable na
saroyanm 2016/04/08 16:56:10 The element is missing, it was removed.
juliandoucette 2016/04/25 19:08:56 This is a standard practice used to identify a val
juliandoucette 2016/04/25 19:08:56 Good point :D . I didn't notice that. I think we
saroyanm 2016/05/06 14:49:27 Acknowledged.
saroyanm 2016/05/06 14:49:28 I agree I'm not sure if it provides much value, bu
juliandoucette 2016/05/11 18:11:06 Will do.
4 if (target.addEventListener)
5 return target.addEventListener(event, callback, false);
6 else
7 return target.attachEvent("on" + event, callback);
8 }
4 9
5 $toTop.click(function () 10 function onLoad(callback)
6 { 11 {
7 jQuery("body,html").animate({ 12 if (document.addEventListener)
8 scrollTop: 0 13 return addListener(document, "DOMContentLoaded", callback);
9 }, 800); 14 else
10 return false; 15 return addListener(window, "load", callback);
16 }
17
18 onLoad(function()
19 {
20 // expand & contract fixed header
21 var header = document.getElementById("header");
22 var headerPadding = 13;
23 addListener(window, "scroll", function()
24 {
25 var scrollY = window.scrollY || document.documentElement.scrollTop;
26 if (scrollY < headerPadding)
27 header.className = "top";
28 else
29 header.className = "";
30 });
31 // open & close header menu (on small screens)
32 var menu = document.getElementById("menu");
33 var menuButton = document.getElementById("header-hamburger");
34 addListener(menuButton, "click", function()
35 {
36 if (menu.className === "open")
37 {
38 menu.className = "";
39 menu.setAttribute("aria-expanded", false);
40 }
41 else
42 {
43 menu.className = "open";
44 menu.setAttribute("aria-expanded", true);
45 }
46 });
11 }); 47 });
12 48 }());
13 var $window = jQuery(window);
14 var $header = jQuery("#header");
15 var headerPadding = 13;
16
17 $window.scroll(function()
18 {
19 if ($window.scrollTop() > headerPadding)
20 {
21 if ($header.hasClass("top"))
22 {
23 $header.removeClass("top");
24 $toTop.show();
25 }
26 }
27 else
28 {
29 if (!$header.hasClass("top"))
30 {
31 $header.addClass("top");
32 $toTop.hide();
33 }
34 }
35 });
36
37 var $menu = jQuery("#menu");
38
39 jQuery('#header-hamberger').click(function()
saroyanm 2016/05/06 14:49:28 Detail: it's "hamburger" also please use double qu
juliandoucette 2016/05/11 18:11:06 Nice catch :)
40 {
41 if ($menu.hasClass("open")) {
saroyanm 2016/04/08 16:56:09 Detail: please move opening curly brace on it's ow
juliandoucette 2016/04/25 19:08:56 Nice catch.
42 $menu.removeClass("open");
43 $menu.attr("aria-expanded", false);
44 }
45 else
46 {
47 $menu.addClass("open");
48 $menu.attr("aria-expanded", true);
49 }
50 })
51
52 });
LEFTRIGHT

Powered by Google App Engine
This is Rietveld