« Plugin revs
New revs - Public comments
Resize images no in Wedge? »

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #570, on March 28th, 2013, 09:28 PM »
See... I figured you might ;)

Now, if only I could figure out why wetem::replace isn't working :/
Posted: March 28th, 2013, 09:09 PM

Never mind, I was misinterpreting what it was supposed to do... though whether it's behaving is another question entirely.

If you tell it to replace a layer, it'll work - the layer's content will be replaced. But if you tell it to replace a block on its own, it will do nothing. I'm not sure if this is intended behaviour or not.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Re: New revs - Public comments
« Reply #571, on March 29th, 2013, 06:13 AM »
So it's now supposed to show all the notifications whether read or unread?
Re: New revs - Public comments
« Reply #572, on March 29th, 2013, 10:34 AM »
@Nao: It seems notification polling is keeping you permanently online. Perhaps updating last activity should be ignored when notifications are being polled? Or remove polling all together?
The way it's meant to be

icari

  • Posts: 88
Re: New revs - Public comments
« Reply #573, on March 29th, 2013, 03:27 PM »Last edited on March 29th, 2013, 04:16 PM by Nao
when you have no notifications and go to (edited out) it is basically a blank page, maybe there should be some text to say you have no notifications?

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #574, on March 29th, 2013, 04:22 PM »
This page is in the process of being rewritten anyway...
Posted: March 29th, 2013, 04:17 PM

@Dragooon, polling should be kept, I like it. Maybe add an option to disable it for either admins (low-quality hosting) or users (e.g. "I'm on a data connection, I have limited bandwidth...")

What I'm working on, right now, is to reduce the size of the response to basically just hold the number of unread messages. And maybe also a boolean saying whether another Ajax request should be made to load notifications when you click the box.
It's all, so very complicated... But interesting, too.
Posted: March 29th, 2013, 04:19 PM
Quote from Arantor on March 28th, 2013, 09:28 PM
If you tell it to replace a layer, it'll work - the layer's content will be replaced. But if you tell it to replace a block on its own, it will do nothing. I'm not sure if this is intended behaviour or not.
What you should do in that context is use wetem::rename.

wetem::replace will take the contents of the layer, and empty it.
wetem::rename will change the name of the layer or block, thus effectively emptying a layer, too. It will also obviously change what template functions are called.

Please tell me if it doesn't work as advertised..? I haven't been checking on it for a long time. Additionally, I'm not sure I even tested replace() at all. (I did test rename, though... Had some fun with it back in the day.)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #575, on March 29th, 2013, 04:24 PM »
replace works as advertised, it replaces the contents of the layer, it just doesn't replace a given block. I didn't realise I was using the wrong function in that case.

The PiePolls plugin uses this to replace the poll template, which is how I discovered it since it needs to drop topic_poll_results and puts its own block in.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #576, on March 29th, 2013, 09:13 PM »
Apologies, I was mistaken: rename() is a layer-only function...
There is no equivalent for blocks.
Should I add support for them...?

live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,670
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #578, on March 29th, 2013, 09:33 PM »
Yup, this is the issue; the pie polls plugin, for example, needs to replace a single block, not an entire layer.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #579, on March 29th, 2013, 09:41 PM »
Done. :)

It was almost too easy... The hardest part was actually finding block names to test the thing :P

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #580, on March 29th, 2013, 11:07 PM »
Quote
- Hmm... I think Pete forgot to remove a code block after rewriting $context['error_new_replies'] to use number_context(). At least, it generated an error for me, and the variable wasn't used anywhere... (Post.php)
It's used in Post2.php. This particular piece of code is a bit of a clusterfuck but ultimately is the result of the fact that the post code can come from different places and go different places depending on what it has.

See Post2.php for $context['new_replies']. lines 277-295 being the most prominent use.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #581, on March 29th, 2013, 11:38 PM »
I meant $txt['error_new_replies'], not $context. And if you'll do a search on error_new_repl, only Post.php has it... Also WedgeDesk, but I'm not going to fix it, it's your 'game' ;)
Anyway, I double-checked before committing. Please revert if you find anything suspicious.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #582, on March 30th, 2013, 12:26 AM »
Eh, I'll revert it and apply the proper fix (because now it's totally broken) next time I commit.

error_new_repl* isn't used... but error_new_replies IS. It's even pulled in just a few lines lower:

Code: [Select]
if ($context['new_replies'] == 1)
$txt['error_new_reply'] = isset($_GET['last']) ? $txt['error_new_reply_reading'] : $txt['error_new_reply'];
else
$txt['error_new_replies'] = sprintf(isset($_GET['last']) ? $txt['error_new_replies_reading'] : $txt['error_new_replies'], $context['new_replies']);

// If they've come from the display page then we treat the error differently....
if (isset($_GET['last']))
$newRepliesError = $context['new_replies'];
else
$post_errors[] = 'new_replies';

$post_errors all get the error_ prefix later. The correct fix would not have been to remove the code (since now it's guaranteed to be broken) as error_new_replies cannot ever be declared properly, but to have replaced error_new_reply with error_new_replies in the $txt declaration.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #583, on March 30th, 2013, 01:54 AM »
I was technically going to modify the language files, but I figured you'd forgotten to clean this up...
My bad ;)

So you're saying I couldn't catch it before the only actual use doesn't specify the error_ prefix, right..?

Time for bed!

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #584, on March 30th, 2013, 05:19 AM »
Quote
So you're saying I couldn't catch it before the only actual use doesn't specify the error_ prefix, right..?
Yup, unless you knew specifically that the post error handler was weird, you wouldn't be able to find it short of doing a test on new_reply and new_replies.

The language files are actually correct on this one, the whole point was to convert multiple strings into one number_context string but since there are two different strings to be pulled from (depending on whether you were on the posting page or quick reply), it just harmonises it all into one $txt entry that doesn't exist prior to the error handler being invoked.
Posted: March 30th, 2013, 03:04 AM

Also
Quote
! Unread Replies feature didn't actually set the participation icon. YES, we know that it's always set, as the page name implies... But still, not showing it when there's a legend next to the change is awkward. (UnreadReplies.php)
That's actually intentional. SMF never did it either. Though in SMF's case there's sort of more of a reason. (SMF didn't separate unread and unread replies into a separate file. One big ass function to do it all. I really need to streamline what we have, thinking about it.)
Posted: March 30th, 2013, 03:06 AM

Also also

http://wedge.org/Themes/default/images/icons/quick_poll.gif as referred to in the legend for a board is missing, and I don't see any poll icon anywhere in the test board itself :/
Posted: March 30th, 2013, 03:30 AM

Also, also also, I should have read a little further up, since yes, I did actually replace the commented bit with the one liner. That'll teach me for trying to review code in a bad mood. Not enough rum (haven't had any in ages, bah)
Posted: March 30th, 2013, 03:49 AM

Also also, also also, some debugging code in the rename method, line 172.
« Plugin revs
Resize images no in Wedge? »