Removed icon font classes from umb-icon implementations

[RachBreeze]

@MMasey implementation of umb-icon means that the font icon classes can be removed and replaced with svgs.

This is done by replacing <i> directive with the umb-icon directive. Hoiwever in a number of places the icon font name has been left in the class, and this is not required. This PR removes the unnecessary classes.

[nathanwoulfe]

Hi @RachBreeze, apologies for taking so long to get to this! I've sorted the merge conflict on this one (our fault for letting your PR sit for six weeks...), and made a couple of updates:

• removed the remaining class="icon" attributes
• refactored some CSS to make everything work after removing class="icon" - since umb-icon adds the .umb-icon class, we can use that instead of adding a new class to the icon element.
• fiddled with the alignment in the scheduled publishing overlay (pic below) - started to go down a rabbit hole, but I've tweaked the UI to make the buttons look like buttons, and get everything sitting on a consistent baseline. It's not perfect, and probably should be overhauled so that the invariant and variant scheduling match, but that certainly warrants a PR of its own.

If you wouldn't mind casting an eye over the bits I changed, then I'll merge if it's all good.

[nathanwoulfe]

@nielslyngsoe fyi - changes to the variant scheduling UI. Per my previous comment too, is there a reason the UI for variant is different to non-variant?

[RachBreeze]

Hi @nathanwoulfe this does look good, the only two thoughts that cross my mind say are that I can't use the keyboard to navigate into the scheduled dates, but I've checked against an older version (8.7.0) and that is true there too (and again would need a separate PR).
And I suspect the older button is dotted because it's not mandatory? But then something is mandatory in order for the schedule button to be enabled? I suspect the older pattern follows the same pattern as other Add Buttons such as Add Content?

But having said that, as I said it all looks good to me :-)

Also thank you for rejigging the CSS and removing class="icon" attributes

[nathanwoulfe]

@RachBreeze would have been helpful too if I'd shared a before shot... Buttons were not styled as buttons, and the alignment was all a bit skewy.

And for comparison, invariant scheduling looks like this:

I'll merge this one, then further changes can be a fresh PR 😄

[bjarnef]

@RachBreeze removing the icon name from the class in <umb-icon> seems to affect the icons for backward compatibility, e.g icon-zoom-out in icon picker.

It used to inherit the font family here, which doesn't seem to work with the legacy icons using font icons and <i> element.

If I add back the class here it inherit the correct font family, otherwise it isn't the icomoon font it inherits (remember custom font packages might call the font family something else than icomoon, e.g. in my package Material Design Icon Pack, where it is named Material-Design-Iconic-Font).

It might not be necessary for to include the icon name when SVG icons are available, but it seems to be necessary to ensure backward compatibility with font icons.

A great way to test this is to install a custom font icon package, e.g. Material Design Icon Pack and in this case search for gmd (as the icons are prefixed with this to avoid collision with other font icon package.

With the latest changes in v8/contrib branch it seems these are not shown correct.

If I add back the icon name to the class, where umb-icon is added the icons are shown again.

Some places it might be okay to remove the class on <umb-icon> but keep in mind the places, where the editor/developer can change the icon and have used a font icon, e.g. in listview layouts, icon picker, tree nodes, etc and that developer could have used e.g. ` in a dashboard or other custom view https://github.com/umbraco/Umbraco-CMS/pull/9255/files#diff-d61bd1bb71f19dc9c6a8ddc611e91493b9c6e57ca9a54cf3f716e794fdf45b18L18

[bjarnef]

@nul800sebastiaan can we revert part of these changes to ensure legacy icons still are shown correct?

[nathanwoulfe]

Maybe we could modify the directive always render the icon attribute into the class as well?

That way the class can be passed for adding additional styles (sizes, colours etc), but we'll know that the icon name is also added as a class. Saves devs having to decide whether or not they need to pass the icon name as class as well.

[bjarnef]

@nathanwoulfe yes, it might be an option the render icon in class attribute here as well https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Web.UI.Client/src/views/components/umb-icon.html#L1

I haven't tested it though.
Maybe a cssClass property as I think @BatJan has added in other components? Then er could render cssClass here, which by default is icon (name) ... if set on component it can join icon + additional values specified.

[bjarnef]

An example here: https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Web.UI.Client/src/views/components/forms/umb-search-filter.html#L2

It can by default set cssClass to icon name and add additional classes.

[nul800sebastiaan]

Thanks all! I've added back the icon name in iconpicker.html to make sure that legacy icons load again. f869259

I see another problem with some search boxes now, the icon is aligned left and pushing the search box down, any ideas?

[bjarnef]

@nul800sebastiaan cool, however it is not only an issue in iconpicker - also if using font icon in component which use <umb-icon> e.g. via <umb-button icon="'my-cool-font-icon'"></umb-button> or e.g. in custom listview layout icon.

yes, I have fixed that as well in #9631

[bjarnef]

The suggestion @nathanwoulfe might work, e.g. something like this.

var directive = {
      replace: true,
      transclude: true,
      templateUrl: "views/components/umb-icon.html",
      scope: {
           cssClass: "@?",
           icon: "@",
           svgString: "=?"
      },
      link: function (scope, element) {
            scope.cssClass = scope.cssClass ? scope.icon " " + scope.cssClass : scope.icon;
            
    }
};

<span aria-hidden="true" class="umb-icon" ng-class="cssClass">
    <span ng-if="svgString" ng-bind-html="svgString"></span>
    <i ng-if="!svgString && icon" class="{{icon}}"></i>
</span>

I haven't tested it though.

[nul800sebastiaan]

Alright, given that we are building a release candidate of 8.11 today, I'm going to have to revert all these changes for now. We'll need to re-attempt this PR with all the problems fixed. Sorry @RachBreeze that it can't be part of 8.11 yet, but we'll get there!

[RachBreeze]

@nul800sebastiaan no worries and I totally understand :-) and thank you