Bug 102315 – Allow WebCoreFullScreenWarningView to animate visibility.

• Component: New Bugs
• Status: NEW

[Jer Noble]

Allow WebCoreFullScreenWarningView to animate visibility.

[Jer Noble]

Created attachment 174317 [details]
Patch

[Benjamin Poulain]

Comment on attachment 174317 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174317&action=review

> Source/WebCore/platform/mac/WebCoreFullScreenWarningView.h:41
> +- (void)show;
> +- (void)showWithFade;
> +- (void)showThenHideWithFade;
> +- (void)hideWithFade;
> +- (void)hide;

Somethings that annoys me a bit with this is the disconnect between those and the hidden properties of NSView.
It would be nice to set the completionHandler of [hideXXX] to hide the view for real, and to unhide it as the first operation of [showXXX].

You are not using all those methods in https://bugs.webkit.org/attachment.cgi?id=174319&action=review
Do you need all of them?

[Jer Noble]

(In reply to comment #2)
> (From update of attachment 174317 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174317&action=review
> 
> > Source/WebCore/platform/mac/WebCoreFullScreenWarningView.h:41
> > +- (void)show;
> > +- (void)showWithFade;
> > +- (void)showThenHideWithFade;
> > +- (void)hideWithFade;
> > +- (void)hide;
> 
> Somethings that annoys me a bit with this is the disconnect between those and the hidden properties of NSView.

How about renaming these to "fadeOut" and "fadeIn"?  I can then override setHidden: to cancel any existing animations.

> It would be nice to set the completionHandler of [hideXXX] to hide the view for real, and to unhide it as the first operation of [showXXX].

Sure.

> You are not using all those methods in https://bugs.webkit.org/attachment.cgi?id=174319&action=review
> Do you need all of them?

The only one not currently used is -(void)show, and with the rename above, it'll go away.

[Benjamin Poulain]

Comment on attachment 174317 [details]
Patch

Sounds good! I r- this one in the meantime.

[Jer Noble]

Created attachment 182807 [details]
Patch

Addressed benjamin's comments.  Will update dependant patches with the new method names.

[Jer Noble]

Created attachment 182861 [details]
Patch

Replaced the completion handler (unsupported in < 10.8) with a simple performSelectorAfterDelay:)

[Andreas Kling]

Comment on attachment 182861 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.

[Jer Noble]

Comment on attachment 182861 [details]
Patch

Yep, still relevant.