HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Active Directory details page

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
detailsdirectorypageactive

Problem

Recently I have been making an MVC application with a fluid, responsive design in mind. One of the views has been to implement a fast responsive active directory details page. This is the first time I've really gone head-first into using jQuery/JavaScript with JSON results. Usually I stick to backend coding and someone else does the front-end using Kendo/Telerik controls. I wanted to change it up a little by using jQuery/JavaScript and HTML5 whereever I can.

The page itself works as follows:

User types into [input:project], which fires the autocomplete to the server, gets the persons/groups details and builds the autocomplete based on results. If the user then selects a person/group from results the details are loaded to the right of the page. If it's a group then I attempt to load the data so it builds the elements on the DOM dynamically and evenly.

Everything works as expected, but I would appreciate your views/comments on my JavaScript here on what I could have done better, if anything. I'm pretty new to jQuery/JavaScript so not sure if I'm writing the functions efficiently etc.

Partial View


































Styles

`
ol, ul {
list-style: none;
}

#project-label {
display: block;
font-weight: bold;
margin-bottom: 1em;
}

#project-icon {
float: left;
height: 32px;
width: 32px;
}

#project-description {

Solution

From a once over,

  • s, v and t are unfortunately named properties of vCard



  • I am confused between the code in vCard.image and getImg, they both call '@Url.Action("GetImageThumb", "Person")', room for de-duping code ? It seems you never call getImg.



  • vCard has data, data retrieval functions and formatting functions. That is too much for 1 object



  • In getPersons you should have function (persons) {



  • You never use var e in getPersons ( consider using JsHint.com ! )



  • You have a number of semicolon warnings in JsHint



  • You should clean up the commented out css lines



  • Using !important sometimes must be done, but it is a code smell. I like how you put / !Important - overrides the inline left of 15px from jquery ui (ul) element/ you should put a similar comment for the other places where you use !important



  • As reader, I see a lot of ( hard to follow ) code that is positioning elements. Automatically I wonder, did the author go all the way trying to solve this with css or did the author take the easy way out ? You should comment why you could not position your elements correctly without JavaScript ( or, try harder to place elements with css )

Context

StackExchange Code Review Q#44655, answer score: 2

Revisions (0)

No revisions yet.