Javascript-форум (https://javascript.ru/forum/)
-   Prototype & script.aculo.us (https://javascript.ru/forum/prototype-script-aculo-us/)
-   -   прошу code review (https://javascript.ru/forum/prototype-script-aculo-us/5599-proshu-code-review.html)

retif 25.10.2009 22:11

прошу code review
 
задача: нужно по нажатии на кнопку разворачивать остальной кусок текста

<div class="showmore">Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.</div>


function showmore() {
	var string_divs = $$('.showmore')
			
	string_divs.length.times(function(n){
		var div = string_divs[n]
		text = div.textContent
		wordArr = $w(text);
		firstPart = wordArr.splice(0,30).join(' ')
		secondPart = wordArr.join(' ')
		console.log(firstPart)
		div.update(firstPart+
				' <span style="display:none;" id="more">'+secondPart+
				'</span><a id="link_more" href="" onclick="moretoggle();return false;">...show more</a>'+
				'<a id="link_hide" style="display:none;" href="" onclick="moretoggle();return false;">hide</a>')
	})
}

function moretoggle() {
	$('more').toggle()
	$('link_more').toggle()
	$('link_hide').toggle()
}

Event.observe(window, 'load', showmore)


вот код решает эту задачу, но мне кажется что можно красивее сделать, есть советы?

mrslndr 18.07.2010 01:04

Цикл times лучше заменить на each или на обычный for (работает быстрее).

Проставить везде ";" где нужно. Это хорошо для ИЕ и вобще хорошая практика, когда-нибудь спасет от неуловимых ошибок.

div.update лучше заменить на div.innerHTML = *; — prototype дает много лишней функциональности в этом методе, в вашем случае лучше обойтись без update, работать будет значительно быстрее. И строки лучше объединять через массив методом join.

Событие onclick лучше поставить на сам .showmore и отслеживать target. Чем меньше событий зарегистрировано в dom, тем лучше.

В итогде получится такой код:

document.observe('dom:loaded', function() {
	var items = $$('.showmore'), item, words, first, second;
	for (var i=0, len=items.length; i<len; i++) {
		item = items[0];
		// Тут ваш код, формирующий разбивку с сылками
		item.innerHTML = [firstPart, '...', second, '...', '...'].join('');
		item.observe('click', moretoggle);
	}
});

function moretoggle(e) {
	var element = Event.element(e);
	if (element.nodeName == 'A' && element.readAttribute('href') === '') {
		Event.stop(e);
		element.select('#more, #link_more, #link_hide').invoke('toggle');
	}
}

mrslndr 18.07.2010 01:07

забыл посмотреть дату поста и написал ответ на сообщение почти годовалой давности)

retif 29.07.2010 17:21

ничего, с удовольствием почитал Ваши комментарии:)


Часовой пояс GMT +3, время: 05:56.