Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Add Splash screen #14837

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[Feature] Add Splash screen #14837

wants to merge 3 commits into from

Conversation

S-H-Y-A
Copy link
Contributor

@S-H-Y-A S-H-Y-A commented Nov 29, 2023

Purpose / Description

Splash screen is one of new Android features and it makes user experiences richer.

This animation is Inspired by the navbar branding image of Anki Web

Fixes

Approach

create splash screen

~android 11 android 12
Screenshot_20231128_223536
screen-20231129-173232.-.Trim.mp4

How Has This Been Tested?

  • physical device (Android 13)
  • emulator on Android Studio (Android 7.1)
    P.S the difference of color of star is already fixed.

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@criticalAY
Copy link
Contributor

Is this really needed that too using a dependency? we already have a splash screen ig

@S-H-Y-A
Copy link
Contributor Author

S-H-Y-A commented Nov 29, 2023

Is this really needed that too using a dependency? we already have a splash screen ig

Ops, this fallback method for android11 and below seems not to depend on androidx.core.core-splashscreen.
I'll remove the dependency within a few hours (because I'm not in home now.)

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this

Lint is failing, can't immediately see why, could you take a look?

From the video, the spinning animation seems much too fast, I'd see about reducing the speed to maybe 0.75x what's currently there.

@S-H-Y-A
Copy link
Contributor Author

S-H-Y-A commented Nov 30, 2023

I can't determine what is wrong because the lint error happens in the code whch has'nt been modified.
I'll try to reset my git commits and recommit.

@S-H-Y-A S-H-Y-A reopened this Nov 30, 2023
@david-allison
Copy link
Member

Thanks for trying, I'll take a look

@david-allison david-allison self-assigned this Nov 30, 2023
@S-H-Y-A
Copy link
Contributor Author

S-H-Y-A commented Nov 30, 2023

From the video, the spinning animation seems much too fast, I'd see about reducing the speed to maybe 0.75x what's currently there.

Thank you for your advice.
I changed the rotation speed, however, both my smartphone and pc are not high end models, and it was found I couldn't record the screen as usual. So, I would like to ask @david-allison to check the animation yourself.

@david-allison
Copy link
Member

The following are unused:

  • Theme_Dark.Launcher.NoTitleBar
  • splash_icon_adaptive.xml

We need to improve our lint error reporting

@david-allison david-allison dismissed their stale review November 30, 2023 15:22

I need to check on my phone

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my phone/emulator, the opposite problem occurs:

There's not enough time to show the 'star' before the splash screen disappears

When I add a sleep, the star spins too quickly

Don't worry about the lint errors for now, I've added this to make them easier to diagnose, should be in main later:

AnkiDroid/src/main/res/drawable/splash_icon.xml Outdated Show resolved Hide resolved
<aapt:attr name="android:animation">
<set>
<!-- Behavior between physical devices and Android Studio preview are different. -->
<!-- <objectAnimator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this commented out code is not required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The animation is correct without The commented out code on physical device, but it is needed for Preview of Android Studio Preview.

@BrayanDSO
Copy link
Member

There's not enough time to show the 'star' before the splash screen disappears

Same here. In those cases, having no animation looks better IMHO

…speed of star of rotation

Star will rotate just once.
@S-H-Y-A
Copy link
Contributor Author

S-H-Y-A commented Dec 3, 2023

On my phone/emulator, the opposite problem occurs:

There's not enough time to show the 'star' before the splash screen disappears

When I add a sleep, the star spins too quickly

Don't worry about the lint errors for now, I've added this to make them easier to diagnose, should be in main later:

I fixed these problems, however, I realized a hack* to rotate star infinitely didn't work well on Android13 and avobe, so I made the star rotate jsut once.

I thought It might be better not to rotate the star.


* This hack is to set
android:repeatMode="restart"
android:repeatCount="infinite"

to objectAnimator.

Splash screen is removed when windowSplashScreenAnimationDuration pass and the system is ready for drawing the view.

On Android 12 the hack works well because windowSplashScreenAnimationDuration is set in style.xml manually.
On Android 13 and avobe, however, the system set windowSplashScreenAnimationDuration 0 automatically and start removing splash screen as soon as it's ready and then splash screen doesn't last until the animation finish.
The reason why it happens is that the system on Android 11 and avobe tries to estimate the duration of the vector animation automatically, but it can't because of the hack.

Due to the bad performance of my smartphone, I couldn't realize that the hack doesn't work.

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this would be cleaner as:

    /** @see executeFunctionAfterSplashScreen */
    override fun onCreate(savedInstanceState: Bundle?) {
        // Note: This is our entry point from the launcher with intent: android.intent.action.MAIN
        executeFunctionAfterSplashScreen(minDurationMs = 1000L) {
            Themes.setTheme(this)
            disableXiaomiForceDarkMode(this)
            val intent = intent
            Timber.v(intent.toString())
            val reloadIntent = Intent(this, DeckPicker::class.java)
            reloadIntent.setDataAndType(getIntent().data, getIntent().type)
            val action = intent.action
            // #6157 - We want to block actions that need permissions we don't have, but not the default case
            // as this requires nothing
            val runIfStoragePermissions = Consumer { runnable: Runnable ->
                performActionIfStorageAccessible(runnable, reloadIntent, action)
            }
            when (getLaunchType(intent)) {
                LaunchType.FILE_IMPORT -> runIfStoragePermissions.accept(Runnable {
                    handleFileImport(
                        intent,
                        reloadIntent,
                        action
                    )
                })

                LaunchType.SYNC -> runIfStoragePermissions.accept(Runnable {
                    handleSyncIntent(
                        reloadIntent,
                        action
                    )
                })

                LaunchType.REVIEW -> runIfStoragePermissions.accept(Runnable { handleReviewIntent(intent) })
                LaunchType.DEFAULT_START_APP_IF_NEW -> {
                    Timber.d("onCreate() performing default action")
                    launchDeckPickerIfNoOtherTasks(reloadIntent)
                }

                LaunchType.COPY_DEBUG_INFO -> {
                    copyDebugInfoToClipboard(intent)
                    finish()
                }
            }
        }
        super.onCreate(savedInstanceState)
    }

    private fun executeFunctionAfterSplashScreen(minDurationMs: Long, function: () -> Unit) {
        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.S) {
            executeFunctionWithDelay(minDurationMs, function)
            return
        }
        splashScreen.setOnExitAnimationListener { splashScreenView ->
            val animationDuration = splashScreenView.iconAnimationDuration
            val animationStart = splashScreenView.iconAnimationStart

            Timber.v("duration :${animationDuration?.toMillis()} start $animationStart")
            val remainingDuration = when {
                animationDuration == null -> 0L
                animationStart == null -> 0L
                else -> (animationDuration - Duration.between(animationStart, Instant.ofEpochMilli(TimeManager.time.intTimeMS())))
                    .toMillis()
            }.coerceAtLeast(0L)
            Timber.v("remainDuration :$remainingDuration")
            executeFunctionWithDelay(remainingDuration, function)
        }
    }
  • simplifying Duration.between(animationStart, Instant.ofEpochMilli(TimeManager.time.intTimeMS()))

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submitted the review too early (only tested on API 33):

The splash screen duration is too long

  • The 'appear' animation needs to be faster
  • the star shouldn't need to spin before it closes

🪲 API 33:

  • reopening AnkiDroid leads to a grey screen

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Dec 8, 2023
Copy link
Contributor

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

@github-actions github-actions bot added the Stale label Dec 22, 2023
@github-actions github-actions bot closed this Dec 29, 2023
@david-allison david-allison removed the Needs Author Reply Waiting for a reply from the original author label Dec 29, 2023
@david-allison david-allison added Keep Open avoids the stale bot Needs a new dev For PR that were good start but the original dev' left. Any dev can take over and finish it. and removed Stale labels Dec 29, 2023
@david-allison david-allison reopened this Dec 29, 2023
@criticalAY
Copy link
Contributor

Would like to take over it

@david-allison
Copy link
Member

Sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Conflicts Keep Open avoids the stale bot Needs a new dev For PR that were good start but the original dev' left. Any dev can take over and finish it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#14837 Add SplashScreen
4 participants