Add optional prop to not dismiss Callout on focus loss#5106
Add optional prop to not dismiss Callout on focus loss#5106jetao-msft wants to merge 1 commit intomicrosoft:5.0from
Conversation
* Add optional prop to not dismiss Callout on focus loss * Change file * Update no-dismiss-callout_2018-06-04-21-44.json
|
|
||
| if ( | ||
| (!this._target && clickedOutsideCallout) || | ||
| !preventDismissOnLostFocus && |
There was a problem hiding this comment.
This definitely shouldn't be here. Instead don't add the focus listener.
There was a problem hiding this comment.
Sure, I can fix that. I was following the pattern of preventDismissOnScroll. Should I give it the same treatment?
Also this is a backfix from #5092 , I can make a follow-up PR on master to do the same thing.
There was a problem hiding this comment.
Actually, looking into how the listeners are registered, is it correct to not register them based on preventDismissOnLostFocus? If _addListeners has already been called, according to the semantics of _hasListeners, it will not be called again. We could enter a situation where you create the Callout with preventDismissOnLostFocus={true}, later update it to false, but the handlers will not be registered.
There was a problem hiding this comment.
hmm. Yeah that's true. For now you're probably right. In the long term it should be streamlined in some way, or perhaps each type of dismiss should have it's own handler. So there would be onFocusDismiss and onScrollDismiss so the user could choose to pass or not pass those dismiss events through. But that should probably be for 7.0
For now keep things the way they are.
There was a problem hiding this comment.
@joschect do you approve this PR then? or are there changes needed?
There was a problem hiding this comment.
OWA has actually upgraded to Fabric 6 in the meantime, so unless we feel this API would be useful for any other Fabric 5 users, I can abandon this PR.
3 participants
Pull request checklist
$ npm run changeDescription of changes
This change adds an optional prop on Callout which, if set to true, prevents the Callout from dismissing itself when it loses focus. This is potentially useful in TeachingBubble cases where the Callout is unprompted and we may want the Callout to stick around until the user explicitly acknowledges it.
This is a backfix to 5.0 from #5092
Focus areas to test
(optional)
Microsoft Reviewers: Open in CodeFlow