The SelectedTemplate and duplicate code

November 11, 2009, 18:14

The SelectedTemplate is used in the MenuList and PageList EPiServer web controls. It is a template used for displaying selected items in navigation lists. This is a nice template to have, but sometimes it is an overkill to use it. Say when you only need to add a class of selected or active (which happened to me today).

The problem

<ul class="nav">
	<li>
		<a href="#">
			<span class="l"></span><span class="c">PageName</span><span class="r"></span>
		</a>
	</li>
	<li>
		<a href="#" class="active">
			<span class="l"></span><span class="c">PageName</span><span class="r"></span>
		</a>
	</li>
</ul>

This is a simple navigation list, but as you can see we have two span tags inside the anchors (this is a technique used when you need to use more than one background image). So we cannot use the EPiServer:Property control that we would use normally.

<EPiServer:Property PropertyName="PageLink" runat="server" />

The normal solution

Instead we’re using a combination of HTML and data binding syntax.

<ItemTemplate>
	<li>
		<a href="<%# Container.CurrentPage.LinkURL %>">
			<span class="l"></span><span class="c"><%# Container.CurrentPage.Property["PageName"].ToWebString() %></span><span class="r"></span>
		</a>
	</li>
</ItemTemplate>
<SelectedTemplate>
	<li>
		<a href="<%# Container.CurrentPage.LinkURL %>" class="active">
			<span class="l"></span><span class="c"><%# Container.CurrentPage.Property["PageName"].ToWebString() %></span><span class="r"></span>
		</a>
	</li>
</SelectedTemplate>

This will work fine, but is not good practice, since we now have duplicated the same code in both templates, only adding class=”active” to the SelectedTemplate.

Solution

We’re now going to update the code to only use the ItemTemplate, and add a little logic for checking if the item should be selected or not.

<ItemTemplate>
	<li>
		<a href="<%# Container.CurrentPage.LinkURL %>" <%# SelectedCssClass(Container.CurrentPage, "active") %>>
			<span class="l"></span><span class="c"><%# Container.CurrentPage.Property["PageName"].ToWebString() %></span><span class="r"></span>
		</a>
	</li>
    </ItemTemplate>

The SelectedCssClass method lives in our code-behind file and looks like this:

protected string SelectedCssClass(PageData page, string cssClass)
{
	var result = string.Empty;
	if (page == null || page.PageLink.ID < 1)
	{
		return result;
	}
 
	if (page.IsSelected(CurrentPage))
	{
		result = string.Format("class=\"{0}\"", cssClass);
	}
 
	return result;
}

As you can see I’m using an extension method (from the EPiCode.Extensions project) in my if statement. This method simply takes the CurrentPage and checks if it, or any of its parents, match the page that is being data binded (same thing as the SelectedTemplate does).

By adding this logic to our code-behind (or a helper class), instead of using the SelectedTemplate just for this little thing, we make it much easier for us or (even more important) the next developer to change and update the navigation list. We’ve eliminated a code smell .

Share:

Tags: EPiCode.Extensions, EPiServer
Category: EPiServer | Comment (RSS) | Trackback The SelectedTemplate and duplicate code

8 Comments

Magne Thyrhaug says:

November 11, 2009 at 19:13

Hi mistar Frederik!

First off: I very much agree. Remove all duplicate code. Good post

As far as code quality goes, I’ve got a thought: There’s nothing regarding the name of the function (isSelected) that states that it should return the a string. The name instead implies that it should return a boolean value – like (I assume) page.isSelected does. Using a more logical name most certainly would make it easier for the next developer

Another thought: What if you want more classes on the element? <a …. class="some-random-class “> maybe?

Yay
Frederik Vig says:

November 11, 2009 at 19:24

Hi Magne!
You’re of course right. I’ve renamed the method to SelectedCssClass, which is a much more descriptive name, and added a second parameter where you can add the CSS class. This is also much more friendly for frontend developers, who otherwise would have to contact the backend developer if they needed to rename the CSS class or add another one.
Eric says:

November 12, 2009 at 09:57

Hi,

A thought: why would you like to create cs-code instead of using templates, except the fact of NOT duplicating code? Instead your are having the same functionality in code-behind forcing a gui-developer to re-compile when he/she only would like to change some style?
Maybe im too much of a gui developer but when creating html and styles I don not want to re-compile the project too often!

Credits to all your great posts!
Frederik Vig says:

November 12, 2009 at 10:55

Hi Eric!
In my experience I mostly end up just adding a class for the selected item. The frontend developer can easily add or change the class by simply changing the second parameter of the SelectedCssClass method (thanks Magne). If the markup and functionality differ much from the regular item, I would use the SelectedTemplate instead, since it makes sense to use in situations when adding everything to the ItemTemplate makes it more complicated and less readable.

You can also use conditional statements
```
<% if(Container.CurrentPage.IsSelected(CurrentPage)) { %>
   
<% }%>
```
I personally find readability more important than having zero duplicated code. So if the regular item and selected item differ much, use the SelectedTemplate. If they are quite similar, like in my example, try to combine them and add an extension or helper method for the logic, but make sure that it does not add difficulties for the frontend developer or others to perform small changes to the code .
Erik Nordin says:

November 12, 2009 at 11:24

One big problem with this approach is if you want to add more css-classes, but I guess you could extend the method for adding this.

I’ve solved this issue by extending the whole PageList and added own templates (if preferred), but I’ve also extended the properties on the Container object with like IsFirst, IsLast, IsSelected, IsAlternating, ItemIndex and ItemCount. That’s all you need to achieve everything Going to blog about it when I have some time over.
Frederik Vig says:

November 12, 2009 at 11:31

Hi Erik!
If you need to add more classes you just add them with spaces between them: < %# SelectedCssClass(Container.CurrentPage, "class1 class2 class3") %>.
I like the idea of extending the PageList and the container object. Looking forward to reading about it .
Erik Nordin says:

November 12, 2009 at 11:40

I mean classes that should be there even though it’s not selected.
Frederik Vig says:

November 12, 2009 at 11:48

Ah sorry, then I would just use something like this:
```
<a href="#"
<% if(Container.CurrentPage.IsSelected(CurrentPage)) { %>
   class="selected">
<% }%>
<% else { %>
   class="notselected">
<% } %>
```
But again, at that point it might be better to use the SelectedTemplate .

Frederik Vig – ASP.NET developer

Follow me