Fix unread marker being removed from DOM

[xPaw]

Fixes #762.

[AlMcKinlay]

Just 1 small issue.

[AlMcKinlay]

If we have the unread marker at the top, and we have 2 date markers straight after, this will not delete 1 of the date markers, will it? Because it'll drop into the else and stop there?

[xPaw]

The intended behaviour is to keep the lowest date marker, does this code not accomplish this?

[AlMcKinlay]

So, if we have:

• unread-marker
• date-marker
• date-marker
• message

We want to end up with

• unread-marker
• date-marker
• message

Correct?

I'm pretty sure this code will not do that, because it will set the first to be unread-marker, then it will check that both 0 and 1 are date-markers, but they aren't. So it will return false. This will then stop it doing the job, I think.

Am I missing something?

[xPaw]

That's correct. And you might be right the code is flawed. Any ideas on how to improve it?

[AlMcKinlay]

Simplest thing I can think of is changing the first one.

Currently we do:

if (!prev) {
 	// Should always be a date-seperator, because it's always added
 	prev = $(this);
} else {
...

So, if we want to make sure that prev is always a date-marker, we can just do this:

if (!prev) {
	if ($(this).hasClass("date-marker")) {
		// We only want to deal with date-markers
	 	prev = $(this);
	}
} else {
...

Then you don't need to add this extra check for the date-marker on the previous one here, as we will always have a date marker on this one.

[AlMcKinlay]

Dodgy indentation.

[AlMcKinlay]

Yep, seems a much better solution.

[AlMcKinlay]

Do we need a second review of this one, or is it small enough to go with 1?

[astorije]

@xPaw is on a roll!! 👏