Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 38 additions & 28 deletions framework/yii/helpers/HtmlBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -1447,42 +1447,52 @@ public static function url($url)
}
}
}

/**
* Adds a CSS class to the specified options.
* If the CSS class is already in the options, it will not be added again.
* @param array $options the options to be modified.
* @param string $class the CSS class to be added
*/
* Check class exits on $options[class]
* @param type $options the html options.
* @param type $class the CSS class to check exist
* @return boolean
*/
public static function cssClassExist($options, $class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to rename it to hasCssClass. Also I think explode can be used instead of regexp.

Or at least PREG_SPLIT_NO_EMPTY instead of extra trim.

{
if (isset($options['class']))
{
$classes = preg_split('/\s+/', trim($options['class']));
return array_search($class, $classes)!==false;
}
return false;
}

/**
* Adds a CSS class to the specified options.
* If the CSS class is already in the options, it will not be added again.
* @param array $options the options to be modified.
* @param string $class the CSS class to be added
*/
public static function addCssClass(&$options, $class)
{
if (isset($options['class'])) {
$classes = ' ' . $options['class'] . ' ';
if (($pos = strpos($classes, ' ' . $class . ' ')) === false) {
$options['class'] .= ' ' . $class;
}
} else {
$options['class'] = $class;
if (!self::cssClassExist($options, $class))
{
if (isset($options['class']))
$options['class'] .= ' ' . $class;
else
$options['class'] = $class;
}
}

/**
* Removes a CSS class from the specified options.
* @param array $options the options to be modified.
* @param string $class the CSS class to be removed
*/
* Removes a CSS class from the specified options.
* @param array $options the options to be modified.
* @param string $class the CSS class to be removed
*/
public static function removeCssClass(&$options, $class)
{
if (isset($options['class'])) {
$classes = array_unique(preg_split('/\s+/', $options['class'] . ' ' . $class, -1, PREG_SPLIT_NO_EMPTY));
if (($index = array_search($class, $classes)) !== false) {
unset($classes[$index]);
}
if (empty($classes)) {
unset($options['class']);
} else {
$options['class'] = implode(' ', $classes);
}
if (isset($options['class']) && self::cssClassExist($options, $class))
{

$options['class'] = trim(preg_replace("/(^|\s)".preg_quote($class)."(\s|$)/", ' ', $options['class']));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why regexp is needed there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say because of the classes boundaries. It must only be replaced if it is found at the start of the string, at the end of the string, or surrounded by whitespace. If you str_replace the class, you might end up replacing parts of other classes (if you don't include spaces in your search), or you might miss those at the start or end (if you do include spaces). Hm. Maybe str_replace( " {$class} ", '', " {$options['class']} ")?

Also, I think the current code produces additional whitespace within $options['class']. one_two_three would become one___three if two was removed, right?

if (empty($options['class']))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yii 2 code style requires using { even for single line if.

unset($options['class']);
}
}

Expand Down