patternjavascriptMinor
Column selection and URL modification
Viewed 0 times
selectioncolumnmodificationandurl
Problem
I have an HTML table representing a database table and a sidebar which holds different anchors ready to perform CRUD operations on click.
Because the hrefs in the anchors aren't in general valid for all columns I have written a script which automatically inserts the id of a selected column into the hrefs:
The code I have written works but it isn't very clean and smart :( Could somebody take a look and say where I have space of improvement?
How could I improve the
EDIT:
As you see when having an available singe object (while editing, showing, deleting) the server side renders the right id/name into the href. Under circumstances when I only have a collection of objects or none object available, jquery has to do the rendering of the right urls. Also some URLs don't require an id (new, index)
Because the hrefs in the anchors aren't in general valid for all columns I have written a script which automatically inserts the id of a selected column into the hrefs:
/localhost/image/var/edit => /localhost/image/4/editThe code I have written works but it isn't very clean and smart :( Could somebody take a look and say where I have space of improvement?
$('table tr').click(function(){
$('.selected').removeClass('selected');
$(this).addClass('selected');
var name = $(this).find('td:nth-child(2)').html();
$('.image_bar ul li a').each(function(){
$(this).attr('href', $(this).attr('href').replace('var', name));
});
});
How could I improve the
td:nth-child(2) selector and the long line modifying the link?EDIT:
{% if image is defined %}
Bilder verwalten
Bild betrachten
Bild hinzufügen
Bild bearbeiten
Bild löschen
{% else %}
Bilder verwalten
Bild betrachten
Bild hinzufügen
Bild bearbeiten
Bild löschen
{% endif %}
As you see when having an available singe object (while editing, showing, deleting) the server side renders the right id/name into the href. Under circumstances when I only have a collection of objects or none object available, jquery has to do the rendering of the right urls. Also some URLs don't require an id (new, index)
Solution
$(function () {
var $lastSelected = $([]);
$('tr').click(function () { //tr tag is always inside a table
$lastSelected.removeClass('selected'); //by caching this variable in the closure we don't need to traverse for it again
$lastSelected = $(this).addClass('selected');
});
$('.image_bar a').click(function (e) {
var url = $(this).attr('href').split('var'), //both checks if var is there and prepares for the replacement
name;
if ($lastSelected.length === 0) {
return url.length==1; //any urls that do not have var in them will still work
}
name = $lastSelected.children().eq(2).html();
window.location = url.join(name);
});
});
Here I am assuming your toolbar is outside of your table.
About the changes:
$(function () {lets you move this script tag elsewhere in your document (perhaps to an external script)
- By caching
$lastSelected, fewer DOM traversals need to happen.
- Knowing
$lastSelectedenables moving the$.fn.eachcall out of the tr click event by setting name inside of it.
- Since the
.eachcall is no longer in the click event, it can be a click event of its own (no more need to iterate over it).
- Instead of modifying the
atags we can just set window.location since we are now in the event where the user has clicked.
- Finally, if a row is not selected up front, the $lastSelected variable will not provide a valid name. Thus we shouldn't allow the link to be clicked if it contains
var(done by the return statement).
Code Snippets
<script type="text/javascript">
$(function () {
var $lastSelected = $([]);
$('tr').click(function () { //tr tag is always inside a table
$lastSelected.removeClass('selected'); //by caching this variable in the closure we don't need to traverse for it again
$lastSelected = $(this).addClass('selected');
});
$('.image_bar a').click(function (e) {
var url = $(this).attr('href').split('var'), //both checks if var is there and prepares for the replacement
name;
if ($lastSelected.length === 0) {
return url.length==1; //any urls that do not have var in them will still work
}
name = $lastSelected.children().eq(2).html();
window.location = url.join(name);
});
});
</script>Context
StackExchange Code Review Q#11309, answer score: 2
Revisions (0)
No revisions yet.