Skip to content

Comments

Updated visibility for buried card counts in Study Options.#18098

Closed
ghost wants to merge 1 commit intomainfrom
unknown repository
Closed

Updated visibility for buried card counts in Study Options.#18098
ghost wants to merge 1 commit intomainfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Mar 15, 2025

Purpose / Description

  • Problem: The Study Options screen incorrectly displays "+0 buried" for New, Learning, and/or Review card categories even when those categories have no buried cards.

  • Impact: This misleadingly suggests there are buried cards when there aren't, confusing users about their study status.

  • Problem: When there are buried cards in the review category, the value displayed is "+0" instead of the correct one. To improve the accuracy and clarity of the Study Options screen, providing users with correct information about their buried cards.

Fixes

Approach

  • Modified the rebuildUi function in StudyOptionsFragment.
  • Added conditional logic to set the text property of the buried card count TextViews (newBuryText, learningBuryText, reviewBuryText) only when there are actually buried cards in the corresponding category.
  • Ensured the buryInfoLabel is visible when there are any buried cards, and invisible when there aren't any.
  • Verified that the visibility of the TextViews are correctly set, hiding them when there are no buried cards.
  • This approach prevents the erroneous display of "+0 buried" by not setting the text property when there are no buried cards.

How Has This Been Tested?

  • This issue has been tested on a physical device running on Android 15, as well as on Android Emulator (API 35 )

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

@ghost ghost closed this Mar 15, 2025
@ghost ghost deleted the solved_18094 branch March 15, 2025 04:52
@david-allison
Copy link
Member

david-allison commented Mar 15, 2025

Thanks! In future can you force push, rather than creating a second pull request. This makes it easier for us to track PRs

@ghost ghost restored the solved_18094 branch March 15, 2025 14:14
@ghost
Copy link
Author

ghost commented Mar 15, 2025

Thanks! In future can you force push, rather than creating a second pull request. This makes it easier for us to track

@ghost ghost reopened this Mar 15, 2025
@ghost
Copy link
Author

ghost commented Mar 15, 2025

Thanks! In future can you force push, rather than creating a second pull request. This makes it easier for us to track

Thank you for the feedback! I understand the preference for the force pushing rather than creating multiple PRs. I initially closed the previous PR because it contained unrelated commits (preference navigation changes), and i wanted to keep this PR focused on the buried card count visibility update.
Currently, there are two PRs for the same issue. Could you please advise on which one should remain open so that I can close the other and ensure proper tracking?

@david-allison
Copy link
Member

I don't mind.

You can rebase & force push either to get rid of the irrelevant commits

@ghost
Copy link
Author

ghost commented Mar 15, 2025

I don't mind.

You can rebase & force push either to get rid of the irrelevant commits

Thank you for the clarification! I'll proceed with rebasing and force pushing to clean up the commits in this PR.

@lukstbit lukstbit added the Needs Author Reply Waiting for a reply from the original author label Mar 16, 2025
@ghost ghost requested a review from ShridharGoel March 16, 2025 14:40
@github-actions

This comment was marked as outdated.

@github-actions github-actions bot added the Stale label Apr 15, 2025
@david-allison david-allison self-assigned this Apr 21, 2025
This refactors a past PR which apparently fixes the issue

I don't feel the PR did fix the issue

The change to use text = "" should resolve this

Fixes 18094

Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com>
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 have force pushed the PR given the review comments

@david-allison david-allison requested a review from lukstbit April 21, 2025 14:11
@david-allison david-allison added Needs Review and removed Needs Author Reply Waiting for a reply from the original author Stale labels Apr 21, 2025
Copy link
Member

@lukstbit lukstbit left a comment

Choose a reason for hiding this comment

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

Thanks David!

@lukstbit lukstbit added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Apr 22, 2025
Repository owner closed this by deleting the head repository Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Second Approval Has one approval, one more approval to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unnecessary "+0 buried" labels are often shown in deck overview screen

4 participants