This repository has been archived by the owner on Feb 20, 2023. It is now read-only.
Thumbnails to wallpapers UI #26671
Merged
Thumbnails to wallpapers UI #26671
+122
−114
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mavduevskiy
force-pushed
the
thumbnails-to-wallpapers-ui
branch
2 times, most recently
from
ff7c960 to
50f2007
Compare
|
This pull request has conflicts when rebasing. Could you fix it @mavduevskiy? 🙏 |
mavduevskiy
force-pushed
the
thumbnails-to-wallpapers-ui
branch
from
f3c2e5a to
f0f1148
Compare
MatthewTighe
reviewed
Sep 6, 2022
app/src/main/java/org/mozilla/fenix/settings/wallpaper/WallpaperSettings.kt
Outdated
Show resolved
Hide resolved
Comment on lines
+465
to
+478
| override suspend fun invoke(wallpaper: Wallpaper): Wallpaper.ImageFileState { | ||
| return if (wallpaper == Wallpaper.Default || fileManager.wallpaperImagesExist(wallpaper)) { | ||
| selectWallpaper(wallpaper) | ||
| dispatchDownloadState(wallpaper, Wallpaper.ImageFileState.Downloaded) | ||
| Wallpaper.ImageFileState.Downloaded | ||
| } else { | ||
| dispatchDownloadState(wallpaper, Wallpaper.ImageFileState.Downloading) | ||
| val result = downloader.downloadWallpaper(wallpaper) | ||
| dispatchDownloadState(wallpaper, result) | ||
| if (result == Wallpaper.ImageFileState.Downloaded) { | ||
| selectWallpaper(wallpaper) | ||
| } | ||
| result |
There was a problem hiding this comment.
I think it would be worth updating the tests to capture the return behavior here.
MatthewTighe
approved these changes
Sep 7, 2022
|
This pull request has conflicts when rebasing. Could you fix it @mavduevskiy? 🙏 |
mavduevskiy
force-pushed
the
thumbnails-to-wallpapers-ui
branch
2 times, most recently
from
2e13561 to
d5e593e
Compare
|
This pull request has conflicts when rebasing. Could you fix it @mavduevskiy? 🙏 |
mavduevskiy
force-pushed
the
thumbnails-to-wallpapers-ui
branch
2 times, most recently
from
7190433 to
f5e0790
Compare
|
|
|
This pull request has conflicts when rebasing. Could you fix it @mavduevskiy? 🙏 |
mavduevskiy
force-pushed
the
thumbnails-to-wallpapers-ui
branch
from
f5e0790 to
c606b2b
Compare
mavduevskiy
added
the
pr:needs-landing
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #26212
Should be merged after this PR.
I made some adjustments to address the bug with
assetFileStatenot updating, that was found here. The original problem with it was that after dispatching new state, items were copied, and further dispatch couldn't rely on the defaultequalsimplementation.But overriding
equalsto comparenamewould preventStore.transitionTofrom updating with the new state, because overriddenequalswill make the new state with the newWallpaperState(with new loading state) to be equal to the old one, hence no update.Also added a new
snackbarto the HomeScreen for edge case when for some reason the app fails to read the file from the storage (disk damage may be? iOS team requested the copywrite for the case, and I following the instructions from here).Moved snackbars from the Compose into the fragment. They wouldn't cancel themselves after a new one has been invoked, observing for failed/success result was easier from the fragment, and they looked quite differently from what we have system wide.
Made changes to the usecase to return the result directly, because calculating the result of the download process from the appstate was complex.
Adds visual feedback to the wallpaper download process.
device-2022-08-29-112717.webm
device-2022-08-30-212856.webm
Pull Request checklist
QA
To download an APK when reviewing a PR (after all CI tasks finished running):
Checksat the top of the PR page.firefoxci-taskclustergroup on the left to expand all tasks.build-debugtask.View task in Taskclusterin the newDETAILSsection.GitHub Automation
Fixes #26212