Problem/Motivation

The tablesort indicator #2840304: Add tablesort_indicator template to utilize glyphicons issue shows an inflexibility of the CSS classes that should be solved. It would be useful if we are also able to add the icon-after and icon-before classes the the icon span itself.

In the theme the icon-after CSS class is missing from the tablesort link, but if we use thead to add margin-left to every glyphicon of a thead this is wrong as people may use glyphicon as column icons without a title.

Not working code:

<a href="/foo" title="Sort by name" class="active">Name<span title="Sort ascending"><span class="icon glyphicon glyphicon-chevron-up" aria-hidden="true"></span></span></a>

What is much more flexible and easier to implement is adding a class icon-after to the icon itself. This can be done in theme_tablesort_indicator().

<a href="/foo" title="Sort by name" class="active">Name<span title="Sort ascending"><span class="icon icon-after glyphicon glyphicon-chevron-up" aria-hidden="true"></span></span></a>

Aside from this theme_tablesort_indicator() need to be able to add something to attributes of bootstrap_icon() to add titles. So we finally should end up with this clean HTML code:

<a href="/foo" title="Sort by name" class="active">Name<span title="Sort ascending" class="icon icon-after glyphicon glyphicon-chevron-up" aria-hidden="true"></span></a>

Proposed resolution

_bootstrap_icon($name, $default = NULL) is prefixed with underscore. So this is officially not a public API function. I'd like to change this to _bootstrap_icon($variables = array()).

Remaining tasks

Write patch.

User interface changes

None

API changes

None. This is an internal function only.

Data model changes

CommentFileSizeAuthor
#6 2844885-6.patch4.14 KBmarkhalliwell

Comments

hass created an issue. See original summary.

hass’s picture

hass commented
Issue summary: View changes
markhalliwell’s picture

markhalliwell
Primary language English
commented

So this is officially not a public API function. I'd like to change this to _bootstrap_icon($variables = array())

No.

This would be a major BC break, regardless if it's "internal" or not (all existing implementations would have to change). I'd rather like to avoid that.

Instead, there should just be an additional $attributes parameter added to the function signature, or better yet.... probably just a single $options parameter that could accommodate for any future growth.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Version: 8.x-3.x-dev » 7.x-3.x-dev
Issue tags: -needs backport to 7.x-3.x

Actually, this isn't needed in 8.x-3.x because Bootstrap::glyphicon returns a render array and one can easily add new attributes using Element::createStandalone, e.g. :

$icon = Element::createStandalone(Bootstrap::glyphicon('chevron-up'))->setAttribute('title', t('Sort ascending'))->getArray();

I'll add the extra $attributes variable to _bootstrap_icon in 7.x-3.x since we're nearing all feature freezes.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Title: Preprocess attributes in bootstrap_icon() » Add ability for _bootstrap_icon() to take default attributes

Better title

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Active » Fixed
StatusFileSize
new 2844885-6.patch4.14 KB

.

  • markcarver committed e3b567a on 7.x-3.x 
    Issue #2844885 by markcarver, hass: Add ability for _bootstrap_icon() to...

  • markcarver committed ce1ff60 on 8.x-3.x 
    Issue #2844885 by markcarver, hass: Add ability for _bootstrap_icon() to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.