CakePHP-Bootstrap / cakephp3-bootstrap-helpers

CakePHP 3.x Helpers for Bootstrap 3 and 4.
https://holt59.github.io/cakephp3-bootstrap-helpers/
MIT License
130 stars 79 forks source link

Add possibility to use FontAwesome icons instead of Glyphicons #152

Closed alphp closed 6 years ago

codecov[bot] commented 6 years ago

Codecov Report

Merging #152 into master will increase coverage by 0.1%. The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #152     +/-   ##
===========================================
+ Coverage     88.91%   89.02%   +0.1%     
- Complexity      368      372      +4     
===========================================
  Files            19       19             
  Lines          1146     1157     +11     
===========================================
+ Hits           1019     1030     +11     
  Misses          127      127
Impacted Files Coverage Δ Complexity Δ
src/View/Helper/HtmlHelper.php 85.53% <100%> (+1.07%) 43 <1> (+4) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ef50df4...8e5de7f. Read the comment docs.

Holt59 commented 6 years ago

Thanks for the PR.

I've intentionally removed font-specific method in a previous version because I think it is up to the user of the helpers to add their own templates.

We are currently working on a dedicated IconHelper to ease the process of customizing the fonts, see issue #149.

For these reason I have to decline the PR in its actual state, feel free to comment on the issue if you have idea regarding the implementation.

alphp commented 6 years ago

I'm not sure that a new helper is needed for the icons. However, I think it is better to change the current HTML helper template for the icons to: 'icon' => '<{{tag}} aria-hidden="true" class="{{font}}{{type}}{{attrs.class}}"{{attrs}}></{{tag}}>', And the icon function could look like this:


   public function icon($icon, array $options = []) {
        $tag = empty($options['tag']) ? 'i' : $options['tag'];
        $fonts['glyphicon'] = 'glyphicon glyphicon-';
        $fonts['fa'] = 'fa fa-';
        $fonts['fas'] = 'fas fa-';
        $fonts['fab'] = 'fab fa-';
        $font = 'glyphicon glyphicon-';
        if (!empty($options['font']) and isset($fonts[$options['font']])) {
            $font = $fonts[$options['font']];
        }
        unset($options['tag']);
        unset($options['font']);

        $options += [
            'templateVars' => []
        ];
        return $this->formatTemplate('icon', [
            'tag' => $tag,
            'font' => $font,
            'type' => $icon,
            'attrs' => $this->templater()->formatAttributes($options),
            'templateVars' => $options['templateVars']
        ]);
    }

The array $fonts could be a property of the extendable class at runtime, such as templates. Although I have no idea how this should be implemented. In this way FontAwesome and Glyphicon would be supported as other sources without modifying the code.

I hope my ideas serve to improve your helpers as much as they have helped me.

alphp commented 6 years ago

If the idea is to create a new helper for the icons I think FontAwesome support should not be removed from the HTML helper until the new ICON helper replaces it.

When the support was removed without an alternative, I caused a small inconvenience, since in my projects I usually use both FontAwesome and Glyphicon. The solution to change the template does not seem appropriate when you switch from one source to another repeatedly, it is not clear and sometimes you do not know what source you are using.

At least I think so.

Holt59 commented 6 years ago

@alphp The goal of the Icon helper is to not rely on the Html helper for the easy-icon feature. We are going for something more generic and more customizable.

The helper will manage a set of templates (possibly with a non-linear organization) so that you would be able to select one easily when needed and to use them with the easy-icon feature.

We will also likely add a special __call or even some runtime created method so that, e.g., faIcon or glIcon would automatically be generated and switch to the appropriate template. This is yet to be decided.

In the mean-time, you can create your own HtmlHelper inheriting the Bootstrap.HtmlHelper to add the features you want (or even faIcon or glIcon methods).