patternjavascriptMinor
Notifications dropdown with TinyScroll
Viewed 0 times
withnotificationstinyscrolldropdown
Problem
I'm relatively new to jQuery and am trying to learn best practices/design patterns with JavaScript. Came across this link and decided to try and put it to something useful.
So far I've made a notifications menu dropdown using TinyScrollBar where the user can remove an individual notification. Once removed, the height of the dropdown updates until there are no more.
Here is a demo of what I have so far.
I would like to know ways on how I can improve my JavaScript.
JS
```
var TEST = TEST || {};
/ Notifications /
TEST.notif = {};
TEST.notif._notifOpen = false;
TEST.notif._el = null;
TEST.notif._sum = 0;
TEST.notif._defaultVPHeight = 350;
TEST.notif.init = function(){
this._el = $('#notifications');
$('.notif-trigger').on('click',function(e){
TEST.notif.toggleNotifications();
return false;
});
// when you click outside the notification menu
// hide the notifications
$('html').on('click',function(e){
if( TEST.notif._notifOpen ) {
TEST.notif.hideNotifications();
}
});
// set initial notifications scrollbar height
this._el.tinyscrollbar( {size: TEST.notif._defaultVPHeight} );
};
TEST.notif.showNotifications = function(){
var $el = this._el,
$close = $('.notif-close'),
$li = $('.notif-list li a');
// show notificatoins and update scrollbar position
$el.show().tinyscrollbar_update();
// stop propagation
$li.on('click',function(e){ e.stopPropagation(); });
// remove list-item
$close.on('click', function(e){
// store
var liHeight, newH, overviewH;
// store individual list-item height
liHeight = $(this).parent().height();
// remove the list-item you clicked
$(this).parent().remove();
// store the new height TEST the viewport
newH = TEST.notif._defaultVPHeight - liHeight;
// get height TEST actual contents
overviewH = $('.overview').height();
// set new defaultH
TEST.notif._defaultVPHeight = newH;
// update scrollbar
$el.tinyscrollbar_update();
// once scrollbar
So far I've made a notifications menu dropdown using TinyScrollBar where the user can remove an individual notification. Once removed, the height of the dropdown updates until there are no more.
Here is a demo of what I have so far.
I would like to know ways on how I can improve my JavaScript.
JS
```
var TEST = TEST || {};
/ Notifications /
TEST.notif = {};
TEST.notif._notifOpen = false;
TEST.notif._el = null;
TEST.notif._sum = 0;
TEST.notif._defaultVPHeight = 350;
TEST.notif.init = function(){
this._el = $('#notifications');
$('.notif-trigger').on('click',function(e){
TEST.notif.toggleNotifications();
return false;
});
// when you click outside the notification menu
// hide the notifications
$('html').on('click',function(e){
if( TEST.notif._notifOpen ) {
TEST.notif.hideNotifications();
}
});
// set initial notifications scrollbar height
this._el.tinyscrollbar( {size: TEST.notif._defaultVPHeight} );
};
TEST.notif.showNotifications = function(){
var $el = this._el,
$close = $('.notif-close'),
$li = $('.notif-list li a');
// show notificatoins and update scrollbar position
$el.show().tinyscrollbar_update();
// stop propagation
$li.on('click',function(e){ e.stopPropagation(); });
// remove list-item
$close.on('click', function(e){
// store
var liHeight, newH, overviewH;
// store individual list-item height
liHeight = $(this).parent().height();
// remove the list-item you clicked
$(this).parent().remove();
// store the new height TEST the viewport
newH = TEST.notif._defaultVPHeight - liHeight;
// get height TEST actual contents
overviewH = $('.overview').height();
// set new defaultH
TEST.notif._defaultVPHeight = newH;
// update scrollbar
$el.tinyscrollbar_update();
// once scrollbar
Solution
From a once over:
-
Review your comments, some of them just take up space and are not very valuable:
Other than that, your code is well structured and easy to follow.
-
Review your comments, some of them just take up space and are not very valuable:
// update scrollbar
$el.tinyscrollbar_update();- Indentation ( this might be because of copy pasting ) is off in your code, making it hard(er) to read
TEST.notifis a mouthful, I would just go fornotifications
_el,_sum,_defaultVPHeightshould all loose their underscore, they dont make sense. Also I would go either for the Spartaneor the full outelement.
TEST.notif._notifOpen-> whynotifOpen, it's already part of an object callednotif, just call itopen
- Use lowerCamelCase consistently:
tinyscrollbar_update->tinyScrollbarUpdate
Other than that, your code is well structured and easy to follow.
Code Snippets
// update scrollbar
$el.tinyscrollbar_update();Context
StackExchange Code Review Q#26355, answer score: 2
Revisions (0)
No revisions yet.