Lots more comments.
- In several places you have incomplete PHP docs like "* @return string".
Elsewhere I have said "* @return string the HTML to be output."
- action_icon should not call $link->prepare(); or $image->prepare();. $this->image and $this->link do that.
- The previous comment applies in lots of other places too. I think you need to review all ->prepare()
- link_to_popup has PHP doc that talks about button.
- link_to_popup should use prepare_url rather than its own code.
- The way you have laid out arrays like $tagoptions = array( does not match the coding guidelines.
- Since we already have JS calling openpopup, we don't need the "this.target='$popup->name'; "; bit. (In several places.)
- I don't think $popup->image makes sense.
- I hate the excessive whitespace indent on comments like (user_picture)
* @param object $userpic Object with at least fields id, picture, imagealt, firstname, lastname
* If any of these are missing, or if a userid is passed, the database is queried. Avoid this
- I don't think the Note: on user_picture is clear. Instead I would write
This method can be used in two ways:
// Option 1:
$userpic = new user_picture();
// Set properties of $userpic
// Option 2: (shortcut for simple cases)
// $user has come from the DB and has fields id, picture, imagealt, firstname and lastname
- I don't think user_picture needs to clone clone($userpic). (But I might be wrong)
- I don't see any benefit in adding class="link" to every link. Just bloat.
- Should help_icon::prepare complain if the user tries to set ->url manually?
- In html_action_component. I think we need to just choose one API, $jsfunction/$jsfunctionargs or $onclick. Giving a choice is just confusing.
- Why isn't html_action_component a html_action_component?
- In html_image, why not just do public $alt = HTML_ATTR_EMPTY; makes prepare() smaller.
- user_picture needs PHPdoc comments. In particualr I don't understand why we need both $link and $url. (You could allow $link to be true (for default link), flase for no link or a url.
Do not take the number of comments as an indication that this patch is bad. Quite the contrary, it is close, but not quite there yet.