Feature request: Rating

[pamidivenkat]

Feature request: Rating : Remove color on hover of stars. Only onclick will fill the star.

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: 6

ng-bootstrap: latest

Bootstrap: latest

[baruchvlz]

Why? It's good UX to show where the mouse is hovering over so the user knows how many stars they're applying. Google, Yelp, Booking, they all do it with their own components.

[pkozlowski-opensource]

We could expose the info to a custom rating template so users would know if fill value comes from hovering or from selection (essentially we could expose 2 fill values). At the same time agree with @baruchvlz - I'm not sure this is the pattern we want to "promote".

Marking as a feature request but don't get too much hope - it is very low priority for us so probably won't get fixed anytime soon (unless someone sends a good PR, of course).

[pamidivenkat]

Why? It's good UX to show where the mouse is hovering over so the user knows how many stars they're applying. Google, Yelp, Booking, they all do it with their own components.

My client feel that sometime its not removing color fill after mouse out. I also observed it sometimes but not able to reproduce always. So they want to get rid of it.

[baruchvlz]

@pkozlowski-opensource Why not have another Input in the component called mode that defaults to hover and checks for the value before updating the state? This way it can also be used with the default rating template and not necessarily depend on a custom template.

@Input() mode: 'hover' | 'selection';

// ...
enter(value: number): void {
  if (this.mode === 'selection') {
    return;
  }

  if (!this.readonly && !this.disabled) {
    this._updateState(value);
  }

  this.hover.emit(value);
}

[benouat]

Looking at this issue, I think it should be fixed in a totally different way, but way that would be a breaking change (so probably available in 5+)

Main idea here would be to not introduce a new @Input(), but more rely on standard behaviour, much closer to what the web platform could provide: using an event to prevent an action.

Let me explain:

We already have today an @Output() hover https://github.com/ng-bootstrap/ng-bootstrap/blob/e0fe9f566061d6a346aab1742f4162ff23e10b70/src/rating/rating.ts#L104-L108

And we use it via the template when we mouseenter a star https://github.com/ng-bootstrap/ng-bootstrap/blob/e0fe9f566061d6a346aab1742f4162ff23e10b70/src/rating/rating.ts#L64-L66

My idea here would be to update the enter() to actually use the hover output as a native event would be used with a preventDefault that would not update the highlighted star.

enter(value: number): void {
    let update = true;
    const event = {
        value,
        preventDefault() {
            update = false;
        }
    }
    this.hover.emit(event);
    if (!this.readonly && !this.disabled && update) {
        this._updateState(value);
    }
}

@pamidivenkat then you would just need to do this

<ngb-rating (hover)="$event.preventDefault()"></ngb-rating>

@pkozlowski-opensource @maxokorokov WDYT ?

If consensus is reached, @baruchvlz I know you worked on the topic, opening #2834, based on your @Input() solution, but would you mind taking care of updating your PR with something like I described ?

[bytelabsco]

@baruchvlz I think a valid reason for being able to disable hover is for mobile safari. For whatever reason, apple has decided that the initial tap on an item with hover will perform the hover event and then requires a second tap to actually select the item. This is actually confusing for iOS users since the hover effect on ngb-rating makes it look like the rating has been changed when it actually has not.

[baruchvlz]

@bytelabsco That can easily be fixed by adding a library like hammerjs (which angular does by default). I've never had issues with tap animations in Safari with Angular.

@benouat I like the idea of keeping the element interactions as close as possible to native elements. I'll be more than happy to adjust the PR once consensus is made.