-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[GSoC'24] Highlight selected preference setting #16368
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested, but I think that this doesn't handle configuration changes (e.g. toggling night mode)
It handles, I consider highlighting in night mode too |
Tested it myself, and it doesn't.
Result: Also, please avoid deleting your own comments. Editing is a more healthy choice for discussions. |
Now I understood what you said. Sorry I didn't test this. I'll make changes accordingly.
Ok will take care of this from next time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current code has two issues:
- on phones(where the majority of our users are) the highlight shouldn't appear as it looks weird. You could fix this by making the Preferences.hasLateralNavigation() method public and then checking its status in highlightHeaderPreference() and return immediately if false(on phones).
- second issue: tablet on any orientation -> highlight one preference -> rotate the tablet -> highlight is gone
AnkiDroid/src/main/java/com/ichi2/anki/preferences/HeaderFragment.kt
Outdated
Show resolved
Hide resolved
hasLateralNavigation function is not included in companion object so we can't access even if we make it public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if it includes the change below.
AnkiDroid/src/main/java/com/ichi2/anki/preferences/HeaderFragment.kt
Outdated
Show resolved
Hide resolved
Also tested on Chromebook |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nitpicks, should be good to go next round
AnkiDroid/src/main/java/com/ichi2/anki/preferences/HeaderFragment.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/preferences/HeaderFragment.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/preferences/HeaderFragment.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers!
Hi there @SanjaySargam! This is the OpenCollective Notice for PRs merged from 2024-05-01 through 2024-05-31 If you are interested in compensation for this work, the process with details is here: Important PLEASE NOTE: The process was updated in August 2024. Re-read the Payment Process page if you have not already. We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but 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! |
Purpose / Description
In tablet mode, the selected settings preference category is not highlighted. It would be better if we highlight the preference to have better user experience.
Approach
Highlight GeneralSettings preference as default
use onPreferenceTreeClick function and remove highlight from previously selected preference and highlight the current preference
How Has This Been Tested?
Emulator
highlight.mp4
Chromebook
Screen recording 2024-05-19 11.47.35.webm
Checklist
Please, go through these checks before submitting the PR.