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

Add `type="button"` to `uif-callout` use of `uif-button` #387

Closed andrewconnell closed 8 years ago

andrewconnell commented 8 years ago

See issue #374 for discussion

jjczopek commented 8 years ago

@andrewconnell: some clarification is needed here. Callout directive itself does not leverage uif-button directive. uif-button is used in demos for the callout, but not on the directive itself. If callout is supposed to have close button, it's added to the markup of the directive on runtime like this:

      let closeButtonElement: ng.IAugmentedJQuery = ng.element(
        '<button class="ms-Callout-close" type="button">' +
        '<i class="ms-Icon ms-Icon--x"></i>' +
        '</button>');

And when it comes to action buttons, I think it should be the responsibility of the user/developer to add type=button if he wants to use uif-button as the action on the callout.

One action point here I believe actually would be to add type=button to callout demos to provide good example to developers.

ghost commented 8 years ago

Looking at the code, and my original post in #374, no change to the directive is required here. Changing the demo would be a good idea though

andrewconnell commented 8 years ago

Agreed... seems like this issue was created but wasn't necessary.