search

ngOfficeUIFabric / ng-officeuifabric

Office UI Fabric (https://github.com/OfficeDev/office-ui-fabric) implementation for Angular
http://ngOfficeUiFabric.com
MIT License
321 stars 67 forks source link

uif-facepile mouse hover issue #427

Open draconnus opened 7 years ago

draconnus commented 7 years ago

Mouse hover on any face in face-pile will change css cursor to Pointer. It should not do that since there is no action to perform on a face in face-pile.

Expected Behavior

Mouse hover over face: cursor: Pointer

Actual Behavior

Mouse hover over face: cursor: auto

Steps to Reproduce Behavior

Hover mouse over face in Basic Usage (http://ngofficeuifabric.com/demos/uifFacepile/)

andrewconnell commented 7 years ago

It shouldn't be hard coded to a Pointer, but it should default to cursor:pointer when not linked. However, when a persona is linked, it should use the finger cursor. That's how it behaves in the Office UI Fabric demos: http://dev.office.com/fabric#/components/facepile

andrewconnell commented 7 years ago

Looked at this for a while and can't figure out where the cursor:pointer is coming from... We aren't using an <a> and the CSS specifies it should use a pointer.

Anyone else want to jump in?

sjclemmy commented 7 years ago

In the Office Fabric (React) version you linked to, the facepile is a div when it isn't a link and a button when it is a link. The css styles come from .ms-Facepile-itemBtn for the div and button.ms-Facepile-itemBtn for the button. The styles for button.ms-Facepile-itemBtn includes cursor:pointer whilst the class .ms-Facepile-itemBtn does not.

The issue is that the styles used for ng-officeuifabric come from a different version. In this instance the class .ms-Facepile-itemBtn DOES contain cursor:pointer.

EDIT: Re-reading, I think there is some confusion on what effect cursor: pointer has. To clarify the meaning of what I have written, cursor: pointer makes the cursor look like a little hand / finger. The default appearance of the cursor is the 'arrow'.

andrewconnell commented 7 years ago

The one I linked to above is the React version that uses the most current CSS... which we don't use. We can't because of conflicts with MDL1 & MDL2 (see #405).

But that does make sense that they switch between using an <a> and <div> when a link is provided or not. Under the covers, we are using our own uif-persona directive for the little circles... I think that's where the underlying control would need to change.

Thoughts?

jjczopek commented 7 years ago

As per @sjclemmy - cursor:pointer comes from the .ms-Facepile-itemBtn class. That's how it is for Office UI Fabric 2.6.2 which we are using (according to the demo on ngofficeuifabric.com).

I also don't think we are able to do anything about it, as at that point of time it was required to wrap persona with this class and thus the pointer.

If we want to align what we have with React implementation, it might be a bigger refactor for persona & facepile components.