Fix background image visibility based on collection state #18073#18162
Merged
lukstbit merged 6 commits intoankidroid:mainfrom Apr 9, 2025
Merged
Fix background image visibility based on collection state #18073#18162lukstbit merged 6 commits intoankidroid:mainfrom
lukstbit merged 6 commits intoankidroid:mainfrom
Conversation
This commit ensures that the background image is correctly shown or hidden based on the collection state. Previously, `currentAnkiDroidDirectory` was being redeclared locally, causing unnecessary shadowing. The fix properly assigns the directory to the existing class-level property and correctly references it. Changes: - Removed redundant local variable declaration for `currentAnkiDroidDirectory`. - Used the existing `lateinit var currentAnkiDroidDirectory` to store the directory path. - Updated the `updateBackgroundVisibility` function to correctly toggle the background image based on whether the collection is empty. Impact: - Prevents redundant memory allocation. - Ensures the background image is hidden when the collection is empty. - Shows the background image when decks are added. Tested to verify the correct behavior of background visibility when decks are added or removed.
Contributor
Author
Screen.Recording.2025-03-26.at.6.37.36.PM.mov |
Re implemented the solution based on the placeholder logic
Member
|
Hi! It's advised that you force push on top of your old commit so that there's only one commit associated with each issue. This way, the commit history of the repo stays clean and descriptive. This was a mistake I made when I first contributed. You can do this by running |
Tushar4059x
force-pushed
the
solving-#18073
branch
from
533d82f to
7dc967a
Compare
Member
david-allison
left a comment
david-allison
left a comment
There was a problem hiding this comment.
Code looks good! A couple of 'line noise' changes in the diff which should be reverted to keep things clean
And a request for a comment change to explain why the line exists
david-allison
added
Needs Second Approval
david-allison
removed
the
Needs Author Reply
david-allison
approved these changes
Apr 1, 2025
Member
david-allison
left a comment
david-allison
left a comment
There was a problem hiding this comment.
Cheers, thanks so much, looks great!
david-allison
added
the
squash-merge
| fabLinearLayout.layoutParams = layoutParams | ||
| } | ||
| } | ||
| Timber.d("Startup - Deck List UI Completed") |
Member
There was a problem hiding this comment.
Wait... this line shouldn't be removed
(missed this on the diff from my notifications)
Contributor
Author
There was a problem hiding this comment.
okay i'll just add it again
david-allison
added
Needs Author Reply
Contributor
Author
|
done |
lukstbit
approved these changes
Apr 9, 2025
Member
lukstbit
left a comment
lukstbit
left a comment
There was a problem hiding this comment.
Ok, thanks for contributing!
lukstbit
added
Pending Merge
github-actions
bot
removed
Pending Merge
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Purpose / Description
Previously, the background image remained visible even when the collection was empty, causing readability issues. This update ensures that the background image is correctly toggled based on whether there are decks in the collection.
Fixes
Approach
How Has This Been Tested?
Checklist
Uploading Screen Recording 2025-03-26 at 6.37.36 PM.mov…