Hi all,

I use Bootstrap sub-theme without CDN (method 1 in documentation).
I didn't understand why glyphicons don't work in my search form.

I found this piece of code:
/theme/boostrap/bootstrap-search-form-wrapper.func.php

// We can be sure that the font icons exist in CDN.
  if (theme_get_setting('bootstrap_cdn')) {
    $output .= _bootstrap_icon('search');
  }
  else {
    $output .= t('Search');
  }

It must be historical, but is it essential now? If some don't use CDN, they won't have search glyphicon... :(
Maybe just remove the if?
Thks!

Comments

markhalliwell’s picture

Title: Glyphicon search without cdn » Implement _bootstrap_icons_supported() helper function and extend _bootstrap_icon() with a $default parameter
Version: 7.x-3.x-dev » 8.x-3.x-dev
Assigned: Unassigned » ryan.armstrong
Issue tags: -glyphicon, -CDN +Needs backport to 7.x

No this isn't historical. It's new in 7.x-3.x. I agree though that this probably isn't the best approach, but it will be a little more difficult to detect when a theme has the Bootstrap icon fonts. We'll likely have to scan a theme's directory to see if they manually implement the font files (ie: support for bootstrap icons). We actually do this in bootstrap_icon_bundles(), but even that needs work so it's not so static. I'll likely need to introduce a helper function that determines whether a theme has Bootstrap based icon support, something like: _bootstrap_icons_supported($theme_key)? I also think we should probably extend _bootstrap_icon() to take an extra parameter so it provides default output (ie: text) if no icon is found: _bootstrap_icon($name, $default = FALSE). So, in theory, we could simplify this entire logic to just:

  $output .= _bootstrap_icon('search', t('Search'));

We should move this to 8.x first and then backport.

markhalliwell’s picture

Title: Implement _bootstrap_icons_supported() helper function and extend _bootstrap_icon() with a $default parameter » Create _bootstrap_icons_supported() helper function

Actually we should split these two into separate issues. There is the child issue which should be addressed after this one has been fixed.

NetOctet’s picture

Thanks for answering. I don't know well enough Bootstrap implementation to help you in code, but i thinks the logic would be better.
Thanks for your work!

I just wonder: glyphicons are part of the module no? Why do you have to check if they exist?

If i use a sub-theme of Bootstrap, and i uncheck CDN port, i don't have to do anything to make glyphicons work (after removing this if), so i don't see why you want to check something...?

NetOctet’s picture

[to be removed, scuse]

markhalliwell’s picture

I just wonder: glyphicons are part of the module no? Why do you have to check if they exist?

If i use a sub-theme of Bootstrap, and i uncheck CDN port, i don't have to do anything to make glyphicons work (after removing this if), so i don't see why you want to check something...?

Because not all sub-themes use the Glyphicons font provided in the Bootstrap framework. By adding specificity and detecting if the files are present, we can assume the icon will be present. If we don't do file checks, the markup for the icon will appear but won't actually show anything because the font files don't exist. If we have Bootstrap icon support we should display an icon, otherwise we should display text/other markup.

The reason it works when the CDN option is checked is because the CDN version supplies the font icons via CSS (@import).

markhalliwell’s picture

Title: Create _bootstrap_icons_supported() helper function » Create _bootstrap_glyphicons_supported() helper function
Issue tags: -Needs backport to 7.x

Committed 529b643 to 7.x-3.x:

Issue #2206725 by Mark Carver | NetOctet: Create _bootstrap_icons_supported() helper function.

markhalliwell’s picture

Committed c28dbc1 to 7.x-3.x:

Issue #2206725 by Mark Carver | NetOctet: Create _bootstrap_glyphicons_supported() helper function.

Fixed local actions text not showing up.

markhalliwell’s picture

Category: Bug report » Feature request
Status: Active » Needs review

  • Commit 529b643 on 7.x-3.x, 8.x-3.x by Mark Carver:
    Issue #2206725 by Mark Carver | NetOctet: Create...
  • Commit c28dbc1 on 7.x-3.x, 8.x-3.x by Mark Carver:
    Issue #2206725 by Mark Carver | NetOctet: Create...

  • Mark Carver committed 529b643 on 8.x-3.x.x
    Issue #2206725 by Mark Carver | NetOctet: Create...
  • Mark Carver committed c28dbc1 on 8.x-3.x.x
    Issue #2206725 by Mark Carver | NetOctet: Create...
markhalliwell’s picture

Version: 8.x-3.x-dev » 7.x-3.x-dev
Assigned: ryan.armstrong » Unassigned
Status: Needs review » Closed (fixed)

I'm just moving this back to 7.x. If this needs re-evaluation in 8.x, create a new issue.