Skip to content

Navigation bar buttons are visible.#20963

Open
Galal-20 wants to merge 2 commits into
ankidroid:mainfrom
Galal-20:Navigation-buttons-designed-to-be-visible
Open

Navigation bar buttons are visible.#20963
Galal-20 wants to merge 2 commits into
ankidroid:mainfrom
Galal-20:Navigation-buttons-designed-to-be-visible

Conversation

@Galal-20
Copy link
Copy Markdown
Contributor

@Galal-20 Galal-20 commented May 6, 2026

Purpose / Description

On some devices, especially Xiaomi devices running MIUI/HyperOS and Android 12 , the navigation bar buttons Back, Home, and Recent were either completely barely visible when the app was in Light Mode.
This happened because when the backdrop was light such as white or light gray, the system did not instantly change the navigation bar icons to a dark color.

Fixes

Approach

  • The fix involves moving away from simply setting the navigationBarColor to using a more robust, logic-based approach:
    • Luminance Detection: Added which calculates the luminance of the background color.
    • Explicit Icon Tinting: Uses if the background is light, this forces the system to draw dark icons, ensuring visibility.
    • Xiaomi Compatibility: Integrated and ensured the navigation bar logic addresses MIUI's specific handling of system UI overlays.

How Has This Been Tested?

  • Android Version: Android 12 , Android 13, and Android 16.
Before After

Checklist

  • You have a descriptive commit message with a short title.
  • 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.
  • UI Changes: You have tested your change using the [Google Accessibility Scanner]

@sanjaysargam sanjaysargam self-requested a review May 10, 2026 11:55
sanjaysargam
sanjaysargam reviewed May 10, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@sanjaysargam sanjaysargam left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @Galal-20 .
Tested in my Xiamo, works perfectly

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt
Comment thread AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt Outdated
Comment thread AnkiDroid/src/main/java/com/ichi2/anki/NoteEditorFragment.kt
@sanjaysargam sanjaysargam added Needs Author Reply Waiting for a reply from the original author and removed Needs Review labels May 11, 2026
@Galal-20 Galal-20 requested a review from sanjaysargam May 11, 2026 12:53
david-allison
david-allison requested changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@david-allison david-allison left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I don't feel the second commit is necessary and I would push to drop it unless you noticed/profiled a significant difference. I suspect it's less than a millisecond in perf for additional code complexity, UI tags etc...

The accessor can stay if you like it. I'd consider defining withInsets { } for even more brevity

Please test on API 29 (Android 10) and API 24.

There are differences in the navigation bar implementation on API 30 and 26

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt Outdated
@Galal-20
Copy link
Copy Markdown
Contributor Author

Galal-20 commented May 12, 2026

I don't feel the second commit is necessary and I would push to drop it unless you noticed/profiled a significant difference. I suspect it's less than a millisecond in perf for additional code complexity, UI tags etc...

The accessor can probably stay if you like it. I'd consider defining withInsets { } for even more brevity

Please test on API 29 (Android 10) and API 24.

There are differences in the navigation bar implementation on API 30 and 26

I have simplified the implementation by removing the caching and UI tags as suggested.
While I was initially concerned about redundant allocations, i agree that the performance impact is negligible and the current approach is much cleaner.
I have also used the withInsets { } and updated its usage across the codebase to improve.

I tested API 29 Android 10 and 24, it's good

pr.mp4

david-allison
david-allison approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
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.

Awesome! Please squash commit 2 and 3

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt
viewModel.showingAnswer.collectLatestIn(lifecycleScope) { isAnswerShown ->
if (isAnswerShown) {
insetsController.hide(WindowInsetsCompat.Type.ime())
withInsets { hide(WindowInsetsCompat.Type.ime()) }
Copy link
Copy Markdown
Member

@david-allison david-allison May 12, 2026

Choose a reason for hiding this comment

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

no need for a single line - with makes sense for multi-line

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt
}

with(requireAnkiActivity().windowInsetsController) {
withInsets {
Copy link
Copy Markdown
Member

@david-allison david-allison May 12, 2026

Choose a reason for hiding this comment

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

Awesome!

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/utils/AndroidUtils.kt
fun FragmentActivity.doOnImeHidden(block: () -> Unit) {
val view = window.decorView
windowInsetsControllerCompat.hide(WindowInsetsCompat.Type.ime())
withInsets { hide(WindowInsetsCompat.Type.ime()) }
Copy link
Copy Markdown
Member

@david-allison david-allison May 12, 2026

Choose a reason for hiding this comment

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

single-line is fine

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/previewer/CardViewerFragment.kt
)
// hide system bars
with(WindowInsetsControllerCompat(window, window.decorView)) {
with(requireAnkiActivity().windowInsetsController) {
Copy link
Copy Markdown
Member

@david-allison david-allison May 12, 2026

Choose a reason for hiding this comment

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

Surprised this isn't using withInsets, but OK

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt
Comment on lines +261 to 266
val insetsController = windowInsetsControllerCompat
if (hasFocus) {
insetsController.show(WindowInsetsCompat.Type.ime())
} else {
insetsController.hide(WindowInsetsCompat.Type.ime())
}
Copy link
Copy Markdown
Member

@david-allison david-allison May 12, 2026

Choose a reason for hiding this comment

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

nit: windowInsetsControllerCompat.show(WindowInsetsCompat.Type.ime()) and remove the var, it's called once on each code path

@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Author Reply Waiting for a reply from the original author labels May 12, 2026
Galal-20 added 2 commits May 12, 2026 20:58
@Galal-20
Navigation bar buttons are visible.
4f982f9 
refactor: Cache and centralize WindowInsetsControllerCompat access
- Use cached extension properties to avoid redundant allocations.
- Ensure efficient reuse during theme changes and UI updates.
@Galal-20
Refactor: simplify insets access and use withInsets extension.
068f90b
@Galal-20 Galal-20 force-pushed the Navigation-buttons-designed-to-be-visible branch from 9caa2b0 to 068f90b Compare May 12, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@david-allison david-allison david-allison approved these changes

@sanjaysargam sanjaysargam Awaiting requested review from sanjaysargam

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Galal-20 @david-allison @sanjaysargam