Sorting icon is not updated

[ctamisier]

After a generation with Angular, in Administration > User Management, sorting on columns doesn't change the icon. It stays on: , no matter the sorting direction.

An attempt of fix is here (but it is not sufficient, maybe a good start?): https://github.com/jhipster/ng-jhipster/pull/73

The bug appeared after the use of angular-fontawesome and/or fontawesome-svg-core. It seems that the underlying svg is not "re-rendered" to display the correct icon.

[fresnault]

What is the current status of this issue ?

@ctamisier Do you have work in progress ? On your side, you just have a problem of rerendered ?

For my part, I just pulled version 5.2.1 and even fa-icon classes are KO.

Step	classes
Initialization	ng-fa-icon
1st click on sort	ng-fa-icon fa-sort-up
2nd click on sort	ng-fa-icon fa-sort-up fa-sort-down
...	ng-fa-icon fa-sort-up fa-sort-down

I can look more closely at the problem if nobody has looked at it.

[pascalgrimaud]

@fresnault : it has been opened 23 days ago with no comment. I think nobody work on it. So you can try to fix it and claim the bounty :-)

[jdubois]

I tried to do it a few days ago and miserably failed :-( That's why I put the bounty on it!

[ctamisier]

For this issue, this PR is (at least) required: https://github.com/jhipster/ng-jhipster/pull/73 It fixes the issue you mentioned to have:

Step	classes
Initialization	ng-fa-icon
1st click on sort	ng-fa-icon fa-sort-up
2nd click on sort	ng-fa-icon fa-sort-down
...	ng-fa-icon fa-sort-up

But then even with this, the style is not rendered... (due to the "js + svg approach" ?)

[deepu105]

I didn't merge that since it still had rendering issues.

On Sat, 8 Sep 2018, 10:20 pm Clément Tamisier, notifications@github.com wrote:

For this issue, this PR is (at least) required: jhipster/ng-jhipster#73 https://github.com/jhipster/ng-jhipster/pull/73 It fixes the issue you mentioned to have: Step classes Initialization ng-fa-icon 1st click on sort ng-fa-icon fa-sort-up 2nd click on sort ng-fa-icon fa-sort-down ... ng-fa-icon fa-sort-up

But then even with this, the style is not rendered... (due to the "js + svg approach" ?)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/8105#issuecomment-419670277, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDlF7MZq97rlHwhOJn6cNjozPSJQA5Nks5uZCZ8gaJpZM4WAgo3 .

[ctamisier]

Totally agree. I just mention it as it might help to solve the problem globally.

[fresnault]

I didn't find a good solution to fix the rendering problem.

Angular-fontawesome renders icon when the change comes from angular binding value, not from classes.

I think there is a interesting response (and example) in this post : https://github.com/FortAwesome/angular-fontawesome/issues/72#issuecomment-408473472 Maybe we can play with the ComponentFactoryResolver (but we need an extra dependency on @fortawesome lib).

Otherwise it is always possible to create a new component like this and show/hide fa-icon according to JhiSort value, but it's really worse than before

@Component({
    selector: 'jhi-sort-icon',
    template: `
        <fa-icon [icon]="'sort'"></fa-icon>
        <fa-icon [icon]="'sort-up'"></fa-icon>
        <fa-icon [icon]="'sort-down'"></fa-icon>
    `
})

[fresnault]

I try to fix it here : generator-jhipster : https://github.com/fresnault/generator-jhipster/commit/3063337c88740367cd81426862fcce7ae60fc399 ng-jhipster : https://github.com/fresnault/ng-jhipster/commit/d852bd05b925255a31449537dd4ee57c1aff771a

That seems to work properly for user-management (angular only). Need more time to test entities generation.

[ctamisier]

That's good that we can have a solution.. But thinking about this issue more globally, it means that having an already rendered 'fa-icon' we are not able to "change it" dynamically by applying different CSS classes. The solution seems that we need to have a component and then re-render it through the Angular way to have this working. Isn't it too restrictive ? (Shouldn't we just remove the js+svg approach (fontawesome-svg-core) ?)

I'm just thinking about people that are already playing with CSS (and not having a dedicated component) to change these kind of icons and it might no longer work for them ?

[fresnault]

I totally agree with you. It was easier by only changing the class css.

I propose a solution that seems to work for sorting the pagination. For the rest, as you say, it's possible to change the icon directly with the angular [icon] directive.

So, it's like you want. There was probably a good reason to have migrated to fortawesome + svg...

[gvandooren]

Hi everybody, To fix this issue, override the jhiSortBy directive and the jhiSortDirective. I try many different fixes but only one is ok. The FaIconComponent implements OnChanges but the changes are never detected.

The fix consists to replace the svg path used to render the icon.

in the sort directive :

private resetClasses() {
        const allThIcons = this.element.querySelectorAll('fa-icon');
        // Use normal loop instead of forEach because IE does not support forEach on NodeList.
        for (let i = 0; i < allThIcons.length; i++) {

            const faIconElement = allThIcons[i];

            this.renderer.setElementAttribute(faIconElement.children[0], 'data-icon', 'sort');
            this.renderer.setElementAttribute(faIconElement.children[0], 'class', 'svg-inline--fa fa-w-10 fa-sort');
            this.renderer.setElementAttribute(faIconElement.children[0].children[0], 'd', this.sortIcon.icon[4]);
        }
    }

in the sortby directive :

    // declare icons
    private sortIcon = faSort;
    private sortAscIcon = faSortUp;
    private sortDescIcon = faSortDown;

private applyClass() {
        let iconName = 'sort';
        let icon = this.sortIcon;
        if (this.jhiSort.predicate === this.jhiAmadeusSortBy) {
            if (this.jhiSort.ascending) {
                icon = this.sortAscIcon;
                iconName = 'sort-up';
            } else {
                icon = this.sortDescIcon;
                iconName = 'sort-down';
            }
        }

        const childIcon = this.el.nativeElement.children[1];
        this.renderer.setElementAttribute(childIcon.children[0], 'data-icon', iconName);
        this.renderer.setElementAttribute(childIcon.children[0], 'class', 'svg-inline--fa fa-w-10 fa-' + iconName);
        this.renderer.setElementAttribute(childIcon.children[0].children[0], 'd', icon.icon[4]);
    }

[deepu105]

@gvandooren could you plz do a PR?

[deepu105]

so that you get the $100 bounty

[gvandooren]

@deepu105 i do that today :)

[lllllpylllll]

I had build ng-jhipster with the code changes from the pull.

It seems that the sorting icon does not response when i tested on Internet Explorer, however Chrome is working fine.

The Internet Explorer - Console is showing the below error ERROR TypeError: Unable to get property '0' of undefined or null reference

[deepu105]

Which version of IE, we actually only support evergreen browsers

On Thu, 8 Nov 2018, 1:32 pm lllllpylllll <notifications@github.com wrote:

I had build ng-jhipster with the code changes from the pull.

It seems that the sorting icon does not response when i tested on Internet Explorer, however Chrome is working fine.

The Internet Explorer - Console is showing the below error ERROR TypeError: Unable to get property '0' of undefined or null reference

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/8105#issuecomment-436978815, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDlF_r4dV9dR4HMREykjitJmSXX44uwks5utCRggaJpZM4WAgo3 .

[lllllpylllll]

IE 11 (Internet Explorer 11)

[lllllpylllll]

Hi deepu105,

i regenerate and reinstall the ng-jhipster that was build locally and it is working fine now.

Thanks

[gvandooren]

@lllllpylllll Thanks for your return :)

[vishal423]

ng-jhipster v0.7 should fix the issue in the main generator.

• ng-jhipster version upgrade is tracked under #9066
• Font awesome dependencies upgrade is tracked under #9079

[scpoole709]

The element has been replaced with an SVGElement which has a className as read-only. To change the appearance you need to change the attribute 'class'. I found that the class names that worked were svg-inline--fa fa-sort, svg-inline--fa fa-sort-up and svg-inline--fa fa-sort-down. Who writes code like this???