From 5d77fc1d2b416d82326ad4aa97399cf9079018c7 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Mon, 28 Dec 2009 19:40:07 +0000 Subject: [PATCH] MDL-21198 improved html_link api, added missing style attribute; marked some potential problems with TODOs --- lib/outputcomponents.php | 40 ++++++++++++++++++++++------- lib/outputrenderers.php | 54 ++++++++++++++++++---------------------- 2 files changed, 55 insertions(+), 39 deletions(-) diff --git a/lib/outputcomponents.php b/lib/outputcomponents.php index e89a4a452fb01..20a277f6e7ec0 100644 --- a/lib/outputcomponents.php +++ b/lib/outputcomponents.php @@ -1269,9 +1269,9 @@ class html_link extends html_component { public $url; /** - * @var string $text The text that will appear between the link tags + * @var string $text The HTML text that will appear between the link tags */ - public $text; + public $text = ''; /** * @var boolean $disabled Whether or not this link is disabled (will be rendered as plain text) @@ -1283,13 +1283,38 @@ class html_link extends html_component { */ public $disableifcurrent = false; + /** + * New link constructor. + * + * @param moodle_url|string $url url of the image + * @param array $options link attributes such as title, id, disabled, disableifcurrent, etc. + */ + public function __construct($url = null, $text = '', array $options = null) { + parent::__construct($options); + + if (is_null($url)) { + // to be filled later + + } else if ($url instanceof moodle_url) { + $this->src = clone($url); + + } else if (is_string($url)) { + $this->src = new moodle_url($url); + + } else { + throw new coding_style_exception('Image can be constructed only from moodle_url or string url.'); + } + + $this->text = $text; + } + /** * @see lib/html_component#prepare() Disables the link if it links to the current page. * @return void */ public function prepare(renderer_base $output, moodle_page $page, $target) { // We can't accept an empty text value - if (empty($this->text)) { + if ($this->text === '' or is_null($this->text)) { // 0 is valid value, do not use empty() throw new coding_exception('A html_link must have a descriptive text value!'); } @@ -1297,9 +1322,10 @@ public function prepare(renderer_base $output, moodle_page $page, $target) { $this->url = new moodle_url($this->url); } - if ($this->url->compare($page->url, URL_MATCH_PARAMS) && $this->disableifcurrent) { + if ($this->disableifcurrent and $this->url->compare($page->url, URL_MATCH_PARAMS)) { $this->disabled = true; } + parent::prepare($output, $page, $target); } @@ -1310,11 +1336,7 @@ public function prepare(renderer_base $output, moodle_page $page, $target) { * @return html_link The link component */ public static function make($url, $text) { - $link = new html_link(); - $link->url = $url; - $link->text = $text; - - return $link; + return new html_link($url, $text); } } diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 4f03e45a6ae9e..e8ab2a4e4cfd4 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -822,48 +822,40 @@ public function block_move_target($target) { * Given a html_link object, outputs an tag that uses the object's attributes. * * @param mixed $link A html_link object or a string URL (text param required in second case) - * @param string $text A descriptive text for the link. If $link is a html_link, this is not required. + * @param string $text A descriptive text for the link. If $link is a html_link, this is ignored. + * @param array $options a tag attributes and link otpions. If $link is a html_link, this is ignored. * @return string HTML fragment */ - public function link($link, $text=null) { + public function link($link_or_url, $text = null, array $options = null) { global $CFG; - $attributes = array(); - - if (is_a($link, 'html_link')) { - $link = clone($link); - - if ($link->has_action('popup_action')) { - return $this->link_to_popup($link); - } - - $link->prepare($this, $this->page, $this->target); - $this->prepare_event_handlers($link); - - // A disabled link is rendered as formatted text - if ($link->disabled) { - return $this->container($link->text, 'currentlink'); - } + if ($link_or_url instanceof html_link) { + $link = clone($link_or_url); + } else { + $link = new html_link($link_or_url, $text, $options); + } - $attributes['href'] = prepare_url($link->url); - $attributes['class'] = $link->get_classes_string(); - $attributes['title'] = $link->title; - $attributes['id'] = $link->id; + $link->prepare($this, $this->page, $this->target); - $text = $link->text; + // A disabled link is rendered as formatted text + if ($link->disabled) { + return $this->container($link->text, 'currentlink'); + } - } else if (empty($text)) { - throw new coding_exception('$OUTPUT->link() must have a string as second parameter if the first param ($link) is a string'); + $this->prepare_event_handlers($link); - } else { - $attributes['href'] = prepare_url($link); - } + $attributes = array('href' => prepare_url($link->url), + 'class' => $link->get_classes_string(), + 'title' => $link->title, + 'style' => $link->style, + 'id' => $link->id); if (!empty($CFG->frametarget)) { + //TODO: this seems wrong, we have to use onclick hack in order to be xhtml strict... $attributes['target'] = $CFG->framename; } - return $this->output_tag('a', $attributes, $text); + return $this->output_tag('a', $attributes, $link->text); } /** @@ -1100,6 +1092,8 @@ public function help_icon($icon) { * @return string HTML fragment */ public function link_to_popup($link, $image=null) { + //TODO: decide if this should be removed completely, because link() already handles this + // we could also add html_link::make_popup() factory method $link = clone($link); // Use image if one is given @@ -1112,7 +1106,7 @@ public function link_to_popup($link, $image=null) { $link->text = $this->image($image); - if (!empty($link->linktext)) { + if (!empty($link->linktext)) { // TODO: this is weird! $link->text = "$link->title   $link->text"; } }