Jump to content

Suggestion: Status::$commentsPerPage variables


Dreadknux

Recommended Posts

Posted

In \IPS\core\Statuses\Status, there's a $commentsPerPageAjax = 25 static variable that doesn't seem to be used anywhere. AFAICT it's intended for the "load more comments" ajax links, but that uses $commentsPerPage = 3 the same way the main page load does.

In addition, there's a static function getCommentsPerPage(void) that just returns $commentsPerPage. There seems to be inconsistent use in code between this function and the static variable itself (mostly the variable), meaning

  • a plugin needs to modify both to avoid buggy behaviour
  • a plugin effectively can't modify getCommentsPerPage() to return the result of an expression (e.g. a setting value), because the static variable (which requires a straight value) needs to have the same value as the function

IMO, $commentsPerPage should only be used in getCommentsPerPage(), which itself should be used in every case that the value is used in code.

  • 4 weeks later...
Posted

The AJAX property is not used, I've removed that to prevent confusion.

Additionally, everywhere should be referencing the method (which allows plugins to override easier and perform logic to determine the number per page), so I've updated the small handful of references pointing directly to the property to use the method instead.

In short: when these changes are released (likely 4.1.12) you can just override the method getCommentsPerPage().

Archived

This topic is now archived and is closed to further replies.

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...