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

Add active class to current menu with extension method or jQuery

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

Problem

In the layout, I have a menu that contains some ul and li.
I have three levels`. This code is a bit of a sample:


     reports
    
        
            text 
            
                text
                 text
            
        
    


To select the current menu item, I use this code:

public bool IsAny(params string[] controllers)
    {
        return controllers.Any(c => Is(c));
    }

    public bool Is(string controller, string action = null)
    {
        if (ViewContext.RouteData.Values["Controller"].ToString().ToLower() == controller.ToLower()
            && (action == null ||
             ViewContext.RouteData.Values["Action"].ToString().ToLower() == action.ToLower()))
            return true;
        return false;
    }


If I use jQuery, is it better than this one on page load speed? Or is there a way to do this in faster way?

The jQuery code looks like this:

(function(){
    var current = 'current location';
    $('#nav li a').each(function(){
        var $this = $(this);
        // if the current path is like this link, make it active
        if($this.attr('href').indexOf(current) !== -1){
            $this.addClass('active');
        }
    })
})

Solution

Regardless of whether you use jQuery or C# to handle the operation it won't have a dramatic effect on page load speed, the only difference will be latency perceived by the user. Because of the manner in which JavaScript operates, you will likely find that the button does not appear as 'active' when the user first loads the page for 0.25 to 0.5s, which may not be a big deal to you, but a user would potentially see that flicker and wonder what they did.

The only changes I would make for you is to rewrite your method just a little to read a bit cleaner:

public bool Is(string controller, string action = null)
{
    if (ViewContext.RouteData.Values["Controller"].ToString().ToLower() == controller.ToLower()
        && (action == null ||
         ViewContext.RouteData.Values["Action"].ToString().ToLower() == action.ToLower()))
        return true;
    return false;
}


To something like the following:

public bool Is(string controller, string action = null) 
{
    var routeData = ViewContext.RouteData;
    var actController = routeData.Values["Controller"].ToString();
    var actAction = routeData.Values["Action"].ToString();

    return actController.ToLower() == controller.ToLower() && (action == null || actAction.ToLower() == action.ToLower());
}


Same number of LoC, but it's a little more clear what's happening.

As far as performance, I don't see any obvious bottlenecks. I think the biggest bottleneck you'll end up having is in the LINQ (this is all speculation, but we're talking microseconds, not even nanoseconds). You shouldn't be concerned of your performance until there's a measurable performance issue.

Code Snippets

public bool Is(string controller, string action = null)
{
    if (ViewContext.RouteData.Values["Controller"].ToString().ToLower() == controller.ToLower()
        && (action == null ||
         ViewContext.RouteData.Values["Action"].ToString().ToLower() == action.ToLower()))
        return true;
    return false;
}
public bool Is(string controller, string action = null) 
{
    var routeData = ViewContext.RouteData;
    var actController = routeData.Values["Controller"].ToString();
    var actAction = routeData.Values["Action"].ToString();

    return actController.ToLower() == controller.ToLower() && (action == null || actAction.ToLower() == action.ToLower());
}

Context

StackExchange Code Review Q#162312, answer score: 3

Revisions (0)

No revisions yet.