Usage of uifDialog

[ghost]

Currently the uifDialog has a few issues which need addressing, listed below. Looping in @jigargandhi as the original author and creator of issue #159 relating to creating services

Opening/Closing dialogs

At the very least, in order to make uifDialog usable, suggest we include uif-is-open to allow consumers to control the state of the dialog from within their app.

Markup

The markup required by consumers is complex, see plunk for how it needs to be used. Suggest a new spec for usage as below

<uif-dialog uif-show-close="true" uif-is-open="true" uif-overlay="dark" uif-type="multiline">
    <uif-dialog-header>Upload profile picture</uif-dialog-header>
    <uif-content>
       <uif-dialog-actions uif-position="right">         
       </uif-dialog-actions>
    </uif-content>
</uif-dialog>

1. removes the need for consumers to wrap the header in a tag
2. close icon and open/closed state is consistent
3. uif-content is consistent
4. reduces the need for consumers to create additional markup

this would be a breaking change, but as uifDialog isn't really usable at the moment, suggest that's OK and it's better to fix before we ship v1.0

[jigargandhi]

I know it is difficult to use uifDialog at the moment due to complex markup. That is the same reason, I have created another issue #159 that allows to create dialogs with simple parameters (similar to $mdDialog service).
@ngOfficeUIFabric/ngofficeuifabric-collaborators please comment.

[andrewconnell]

If it's complicated to use the dialog, then we should fix that if possible, not create something else to fix it. I get the idea of a service, but we should fix the issue if there's an issue (I don't have experience in using the dialog so can't speak to it).

[ghost]

IMO, a service is overkill, we're not entirely consistent in other directives which require layering and/or showing and hiding. This may be because some can use ng-show or ng-hide, whilst others are more complex (uifPanel does animation in and out for instance).

• uifMessageBanner using uifIsVisible
• uifMessageBar has no show/hide implementation
• uifOverlay has no show/hide implementation
• uifDialog has no show/hide implementation
• uifPanel uses uifIsOpen

I realise we're going off piste here, but I think we have two options:-

1. Improve documentation to demonstrate showing and hiding using native angular methods where appropriate
2. Use a consistent method to show and hide uif directives through the use of an attribute like uifVisibile, accepting there is an overhead for developers but may make the user experience a little more pleasant.

Would welcome thoughts

[jigargandhi]

as per comment from @tobiaswest83 it seems that different directives use different attributes.
I am open to use either uifIsVisible or uifIsOpen.
May be uifDialog is difficult to use and we should definitely fix it by using one of the above attributes.
Which one should be used?
@ngOfficeUIFabric/ngofficeuifabric-members please vote.

[andikrueger]

I would vote for an improved documentation.

e.g. uifMessageBanner using uifIsVisible provides more information about the status of the message bar. The Message bar's controller sets the value of the uifIsVisible scope variable (two-way-binding) and therefore reflects the user's close-event .

[andrewconnell]

Seems this one has stalled out... restarting the discussion to reach a resolution...

consistent open / close across all directives

This is a bigger topic, but one we should standardize on. I personally think the ng-show and ng-hide should be used, but not within the directive... this should be something the user could implement. The reason for this is maybe they want the component to disappear while in other places they want to use an animation. Either way, they could have control over how it's shown & hidden... if the directive included this logic within it, it would force the user to use their implementation. This would require touching with likely breaking changes on directives that already have implementations (uif-message-banner & uif-panel).

simplifying the markup

While I get what @tobiaswest83 proposes, the header should be wrapped in a tag so the user can control what the header looks like. Or am I mistaken that you could control it... for instance, include text & an image in the header and control the CSS class used to style the text like the current one uses.

I do think the uif-dialog-inner could be removed... the dialog could just have three nested pieces:

<uif-dialog>
  <uif-dialog-header />
  <uif-dialog-content />
  <uif-dialog-actions />

Thoughts?

[jigargandhi]

After short research, I found that .ms-dialog has a property of font-size:0 specified within it.
Due to this, we are not able to use the ms-Dialog-header, ms-Dialog-content or ms-Dialog-actions.
See this plunk for details where the p tag mentioned inside the .ms-Dialog-content.

If we simplify the markup currently, user will have to

1. Know that the font-size is 0
2. Set the font size explicitly.

If the font-size:0 is removed, we will than be able to simplify the markup as suggested by @tobiaswest83.

Is this a bug that should be logged in the Office UI Fabric?

[andrewconnell]

@jigargandhi Strange... maybe post a question as to why they do that with Office UI Fabric...

[jigargandhi]

Added an issue in OfficeUI repo.

[andrewconnell]

I'm going to close this issue... now that we have an actual spec and working example from the Office UI Fabric guys @ MSFT and a working React version, that should be used as the spec.

See #455.