Icons should be added via CSS

[slavafomin]

Right now icons are added to buttons as character symbols:
https://github.com/isteven/angular-multi-select/blob/master/isteven-multi-select.js#L898-L900

This is a limitation.

Actually, the visual part of the component should be controlled with CSS for ease of customization.

I will try to do a PR on this.

[slavafomin]

Please see the PR.

I've removed icons from JS and added them via :before pseudo-element in CSS.

[isteven]

Hi @slavafomin ,

I put the icons there in the code so people can easily use HTML tags if they want to (For example, using font-awesome, Bootstrap's glyphicon, or perhaps a custom image if they want to).

[slavafomin]

Icons actually are not in HTML template, they are in the JavaScript code, which is a bad practice. We should try to separate things as much as possible. Especially, we shouldn't mix business logic of the component with it's presentation.

And, by the way, we can use FontAwesome pretty easy using CSS, we don't actually need to use explicit elements like span.fa.fa-icon-name. This can be achieved using SASS or LESS. And if the user will decide to add icon elements directly to the template he will be able to override the template and add new elements explicitly.

---

If you were talking about adding icons by means of specifying translation string with HTML components in it:

1. It won't work currently (cause pre-defined icon is always appended to the translation string).
2. We will end up with mixed concepts again (we shouldn't mix translation with presentation).

[isteven]

Hi @slavafomin ,

Your idea is conceptually correct. Do you have a suggestion on how to do this in practice?

The goal is to:

• Still allowing users to define their own style and icon (be it image, font awesome etc)
• Library agnostic, using only AngularJs and nothing else.
• Directory / folder agnostic.

[slavafomin]

Hey @isteven.

I think the first order of business is to remove icon-related code completely from JS and move it to CSS and/or to HTML.

Please see my PR #246 — I've removed icons from JS and moved it to CSS. I've had to add some CSS classes to buttons in order to better control it from CSS side.

I think it achieves the desired goals:

1. Users can define their own icons either by overriding CSS styles or by overloading the entire template.
2. I've not added any dependencies.
3. What do you mean by directory agnostic? I think this problem has nothing to do with filesystem structure cause all paths are controlled by CSS.

If you will agree to accept the PR I will be glad to write small documentation on how to use custom icons.

[isteven]

Hi @slavafomin ,

Correct me if I'm wrong - the CSS solution you mentioned works on Unicode icons. What if a user want to use font-awesome, or bootstrap's glyphicon, or custom image, or no icons at all?

[slavafomin]

Not using icons at all is easy, you will just need to add content: ''; to your CSS.

If you want to use FA icons you have several options:

1. If you are using vanilla CSS you can just copy icon definitiion from provided FA's CSS file and add it to the button via some pseudo-selector, e.g. :before in your CSS. This will work, but it's not DRY and increases the size of CSS file. Also it will break future compatibility with FA. I will recommend to avoid this approach if possible.
2. If you are using some CSS compiler like SASS/Compass or LESS (and you definitely should!) you can easilly include FA's mixins to your CSS file. This will help you to avoid DRY problem as well as future compatibility problem. This is a pretty solid approach.
3. You can overload the component template and explicitly add FA's elements to the HTML. This will give you the highest possible level of customization and will mitigate all mentioned problems.

Also, we can split component's template into several smaller templates. That way user will not have to overload the entire template, just some part of it.

[isteven]

@slavafomin ,

Thanks for the ideas - you do have pretty solid knowledge in CSS 👍

I personally don't prefer solution 1) for the same reason that you mentioned. It's really not practical for both short and long term.

Solution 2) is ok, but then again I want the directive to be library agnostic (this has been one of the primary goals of the directive since day 1).

This leaves us with option 3), that is overloading the template. Like you said, this will allow the user to customize things to the highest level, and avoid all the problems. But tell me, is it practical for the users? I'm an old school developer here, and usually, if I want to modify small things like icons, I'll straight away open the code (be it HTML template, CSS or JS), search the thing I want to change, update and save. I don't have to write & inject any extra code etc. What is the effort to overload template (as in doing it AngularJs way), to get the result that we want (in this case, changing the icons)?

[slavafomin]

I think you've misunderstood no. 2.

I'm not asking you to add CSS to the library. I was speaking about the end-user. She will be able to customize the library buttons by including FA's mixins in her CSS. No need to touch the library or to overload the template. I think this is a compromised solution.

Considering option No. 3, if you prefer, we can add explicit icon-elements to the default HTML template. The end user will be able to just replace classes for this elements to make it FA or something else.

E.g. we can have such button in default template:

<button class="button button-select-all" type="button">
    <span class="icon icon-select-all"></span>
    {{ selectAllTitle }}
</button>

User will be able to use FA icon by replacing it with:

<button class="button button-select-all" type="button">
    <span class="fa fa-select-all"></span>
    {{ selectAllTitle }}
</button>

The difference will be in class="icon icon-select-all" vs class="fa fa-select-all".

Or, we can even add configuration option to override icon classes, e.g.:

var defaultIcons = {
  selectAll: 'icon icon-select-all',
  selectNone: 'icon icon-select-none',
  // ...
};

User will be able to override those:

<div
    isteven-multi-select
    icon-classes="myIconClasses"
>
</div>

$scope.myIconClasses = {
  selectAll: 'fa fa-select-all',
  selectNone: 'fa fa-select-none',
  // ...
};

And the template will be:

<button class="button button-select-all" type="button">
    <span class="{{ iconClasses.selectAll }}"></span>
    {{ selectAllTitle }}
</button>

I think I will be able to make a PR for this if you'd like.

[nathanboktae]

+1 for keeping styling concerns in CSS. Then users can use font icons, background images as raster or SVGs, etc. as the choose.

[brycehewett]

+1 for keeping the icons in css.

[Amenel]

This is a sound proposal that adds value to the library without removing anything. Has the PR been merged?

[jeroenheijmans]

I was about to open a similar issue, as our designer had effectively forked the lib merely for the purpose of changing the icons.

How about an approach similar to how you've done the "translation" bit? Allow me to express my suggestion in (pseudo)code:

$scope.icon        = {};
if (typeof attrs.iconCustomization !== 'undefined') {
    $scope.icon.selectAll       = $sce.trustAsHtml( $scope.iconCustomization.selectAll );
    $scope.icon.selectNone      = $sce.trustAsHtml( $scope.iconCustomization.selectNone );
    $scope.icon.reset           = $sce.trustAsHtml( $scope.iconCustomization.reset );
    $scope.icon.tickMark        = $sce.trustAsHtml( $scope.iconCustomization.tickMark );
} else {
    $scope.icon.selectAll       = '✓';    // a tick icon
    $scope.icon.selectNone      = '×';     // x icon
    $scope.icon.reset           = '↶';     // undo icon            
    $scope.icon.tickMark        = '✓';    // a tick icon 
}

I've used / mimicked the existing style of the lib with the above, making it analogous to the translation section.

Obviously the iconCustomization should be bound on the scope as well.

This allows users to override the default html entities with either other entities, tags with icons in a font of their choosing, or heck: even img tags.

PS. If a change like this happens, the hard-coded    bits on the translation may also have to be dealt with: our use-case was to set the icons to nothing at all (empty strings), but then those two non-breaking spaces will still be rendered, causing awkward alignment. For that I'd suggest changing the translation bit so that it special-cases falsy icon settings (for backwards compatability; if you can do breaking changes removing the hard-coded non-breaking spaces in favor of a css-based solution would be preferred).