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

Panel: clicking on overlay shouldn't automatically close the Panel (enhancement). #434

Closed sjclemmy closed 7 years ago

sjclemmy commented 7 years ago

Desired Behavior

Provide boolean option to indicate whether clicking on the overlay should automatically close the Panel.

I'm happy to do the PR.

andrewconnell commented 7 years ago

While they are focusing on React, I think that the behavior & public signature of our directives in this version of the library (and future versions) should mirror the public signature & behavior of the official Office UI Fabric, like this for panel: http://dev.office.com/fabric#/components/panel

Does theirs behave the same way ours does? If not, we need a solid use case as to why we should deviate.

sjclemmy commented 7 years ago

Their React implementation is very different with regards to the public signature. They provide an isLightDismiss boolean property that is false by default. When set to true, clicking on the overlay closes the panel: https://github.com/OfficeDev/office-ui-fabric-react/blob/master/packages/office-ui-fabric-react/src/components/Panel/Panel.tsx#L110.

To reflect the default behaviour of the React component in this library the line https://github.com/ngOfficeUIFabric/ng-officeuifabric/blob/master/src/components/panel/panelDirective.ts#L26 should be removed.

To reflect the public signature the boolean operator isLightDismiss and related behaviour should be implemented.

For my use case, I just want to disable the clicking of the overlay to close the panel, so for speed would prefer to just remove the ng-click. However this may cause breaking changes for existing implementations.

I am happy to do a PR that implements the isLightDismiss property and sets it to false by default.

andrewconnell commented 7 years ago

LGTM... if you'd like to tackle this one and submit a PR with the changes you outline, I'm game.

To the point about their public signatures being different from ours... that isn't surprising and frankly to be expected. We started this project when they were just CSS with some example JS, but since then they started with a whole React version that they needed for the rest of SharePoint & Office.

From my POV (and curious what the rest of the team thinks @ngOfficeUIFabric/ngofficeuifabric-members), since they have their public signature documented, seems like we'd want to do the same. Will provide a good experience for developers looking to consider an Angular (ng1 or ng2) implementation for their Office related projects when they don't want to use React. Sure... this will mean we have a LOT more work to do... I'll add this as part of my audit for v1.

ghost commented 7 years ago

sorry been so slow on this guys, various factors. I'll leave it with you @sjclemmy

edit getting wires crossed here, would agree with @andrewconnell here about marrying up the public sigs :)

andrewconnell commented 7 years ago

You referring to #373 @tobiaswest83 ?