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

fix(deck-options): add progress bar to stop flickering #17891

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

david-allison
Copy link
Member

@david-allison david-allison commented Jan 28, 2025

Purpose / Description

This is the last of a series of 3 pull requests:

  • refactors which can go in before the backend bump
  • bump the backend and fix Deck Options
  • (we are here) use the new backend functionality to fix Deck Options loading

Fixes

Approach

deckOptionsReady allows us to know exactly when the backend is ready, instead of showing the WebView in PageWebViewClient.onPageFinished.

Stop showing the WebView in onPageFinished, and wait for deckOptionsReady

A CircularProgressIndicator has been added as the wait is noticeable (~300ms)

Once the WebView is shown, hide the Progress Bar

promiseToWaitFor and :page-fully-loaded are also removed as unused, and these were primarily for DeckOptions

How Has This Been Tested?

Personal S21 (Android 14)

Checklist

  • 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

@david-allison david-allison added Needs Review Blocked by dependency Currently blocked by some other dependent / related change labels Jan 28, 2025
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

seems good - only query is about handling e-ink, I think for them just no indicator and 300ms is...I dunno ...good performance for an e-ink device? so it's fine to just let it sit for a moment before webview is visible?

Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Ah, cool re: respecting system settings. I guess this is a benefit of "not fighting the platform", or stated positively: "using platform built-in features", they already do so much of what you want. LGTM then

`deckOptionsReady` allows us to know exactly when the backend is ready,
instead of showing the WebView in PageWebViewClient.onPageFinished

Stop showing the WebView in `onPageFinished`, and wait for `deckOptionsReady`

A `CircularProgressIndicator` has been added as the wait is noticeable (~300ms)

Once the WebView is shown, hide the Progress Bar

promiseToWaitFor and `:page-fully-loaded` are also removed
as unused, and these were primarily for DeckOptions

Fixes 14194 (better): deck options no longer flicker
@david-allison david-allison removed Blocked by dependency Currently blocked by some other dependent / related change Has Conflicts labels Jan 28, 2025
@mikehardy mikehardy added this pull request to the merge queue Jan 28, 2025
Merged via the queue into ankidroid:main with commit a08be69 Jan 28, 2025
9 checks passed
@github-actions github-actions bot added this to the 2.21 release milestone Jan 28, 2025
@david-allison david-allison deleted the anki-25-01-post branch January 28, 2025 15:47
@david-allison
Copy link
Member Author

Hi there @david-allison! This is the OpenCollective Notice for PRs merged from 2025-01-01 through 2025-01-31

Starting 2025, we have updated our selection process for Open Collective, and you are eligible for compensation for your work, the process with details is here:

https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

This note applies to all PRs merged for this month.

Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deck Options: White screen flicker in night mode
2 participants