Skip to content
This repository has been archived by the owner on Jun 14, 2018. It is now read-only.

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

Closed
sjclemmy opened this issue Dec 12, 2016 · 5 comments

Comments

@sjclemmy
Copy link
Contributor

sjclemmy commented Dec 12, 2016

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link

ghost commented Dec 12, 2016

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
Copy link
Member

You referring to #373 @tobiaswest83 ?

sjclemmy added a commit to sjclemmy/ng-officeuifabric that referenced this issue Dec 12, 2016
Add uif-is-light-dismiss as an attribute of the Panel directive to allow the setting to determine whether clicking on the background overlay closes the Panel. Default value is false. When not set, or set to false, clicking on the background overlay has no effect. When set to true, clicking on the background overlay closes the Panel.

Closes ngOfficeUIFabric#434
andrewconnell pushed a commit that referenced this issue Dec 14, 2016
Add `uif-is-light-dismiss` as an attribute of the Panel directive to allow the setting to determine whether clicking on the background overlay closes the panel. Default value is false. When not set, or set to false, clicking on the background overlay has no effect. When set to true, clicking on the background overlay closes the panel.

Closes #434.
andrewconnell pushed a commit that referenced this issue Dec 14, 2016
Add `uif-is-light-dismiss` as an attribute of the Panel directive to allow the setting to determine whether clicking on the background overlay closes the panel. Default value is false. When not set, or set to false, clicking on the background overlay has no effect. When set to true, clicking on the background overlay closes the panel.

Closes #434.
andrewconnell pushed a commit that referenced this issue Dec 14, 2016
Add `uif-is-light-dismiss` as an attribute of the Panel directive to allow the setting to determine whether clicking on the background overlay closes the panel. Default value is false. When not set, or set to false, clicking on the background overlay has no effect. When set to true, clicking on the background overlay closes the panel.

Closes #434.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants