Bug 139715

Summary: Implement and adopt two new TextIndicator presentation animations
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, commit-queue, eric.carlson, glenn, jer.noble, philipj, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch andersca: review+

Description Tim Horton 2014-12-16 17:55:37 PST
Implement and adopt two new TextIndicator presentation animations
Comment 1 Tim Horton 2014-12-16 17:57:28 PST
Created attachment 243413 [details]
Patch
Comment 2 Tim Horton 2014-12-16 18:34:45 PST
Created attachment 243420 [details]
Patch
Comment 3 Anders Carlsson 2014-12-16 18:35:36 PST
Comment on attachment 243413 [details]
Patch

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

> Source/WebCore/page/mac/TextIndicatorWindow.mm:246
> +    return _textIndicator->presentationTransition() == TextIndicatorPresentationTransition::BounceAndCrossfade || _textIndicator->presentationTransition() == TextIndicatorPresentationTransition::Bounce;
> +}

Please use a switch statement here instead.

> Source/WebCore/page/mac/TextIndicatorWindow.mm:251
> +    return (_textIndicator->presentationTransition() == TextIndicatorPresentationTransition::BounceAndCrossfade || _textIndicator->presentationTransition() == TextIndicatorPresentationTransition::Crossfade) && _textIndicator->contentImageWithHighlight();
> +}

Please use a switch statement here instead.

> Source/WebCore/page/mac/TextIndicatorWindow.mm:256
> +    return _textIndicator->presentationTransition() == TextIndicatorPresentationTransition::FadeIn;
> +}

Please use a switch statement here instead.

> Source/WebCore/page/mac/TextIndicatorWindow.mm:372
> +    if (m_textIndicator->presentationTransition() == TextIndicatorPresentationTransition::Crossfade || m_textIndicator->presentationTransition() == TextIndicatorPresentationTransition::FadeIn)
> +        startFadeOut();
> +    else
> +        closeWindow();
> +}

Switch statement?

> Source/WebCore/page/mac/TextIndicatorWindow.mm:412
>      if (m_textIndicator->presentationTransition() != TextIndicatorPresentationTransition::None) {

Single-line if statement.
Comment 4 Tim Horton 2014-12-16 19:02:31 PST
http://trac.webkit.org/changeset/177427