ankidroid / Anki-Android

AnkiDroid: Anki flashcards on Android. Your secret trick to achieve superhuman information retention.
GNU General Public License v3.0
8.54k stars 2.21k forks source link

Phone's control bar white/light in dark mode #5409

Closed tomas-hartman closed 4 years ago

tomas-hartman commented 5 years ago
Issue

Hi, I encountered an issue with the way the control bar (where the phone control buttons are located) is displayed in night mode on my Samsung phone (android 9/one ui). In most other apps, color of the bar is black, however in AnkiDroid, its color is displayed as white/light which creates a really annoying contrast with the black background of the app itself, especially (but not only) during night time. Except for the uncomfortable conditions it makes, it can also have some negative effect on battery life on AMOLED displays.

Screenshots

black bar in other apps light bar in anki @ night mode

Research

Enter an [ x ] character to confirm the points below:

[ x ] I have read the support page and am reporting a bug or enhancement request specific to AnkiDroid

[ x ] I have checked the manual and the FAQ and could not find a solution to my issue

[ x ] I have searched for similar existing issues here and on the user forum

mikehardy commented 5 years ago

Ha - I see you already logged an issue. Is this with the latest beta?

tomas-hartman commented 5 years ago

Hi, nope it's the production version from Google Play. I'll join beta program and let you know. :)

tomas-hartman commented 5 years ago

OK, still the same with 2.9beta5.

mikehardy commented 5 years ago

Bummer - but thank you for testing it

Kartikkman commented 5 years ago

Hii @mikehardy ,

I want to contribute towards this project . I am a beginner in this regards . Should I proceed with this issue ??

Thank You

mikehardy commented 5 years ago

Sure @Kartikkman - and welcome! Check our wiki for info on Development (and note that apparently you need to turn Instant Run off in Android Studio per #5406)

That should get you all set up so that you compile the current code and having it run on an emulator on your machine, at that point you're ready to start making changes

This change specifically? I am guessing (could be wrong) that there will be some sort of XML style attribute we can set? Probably something missing in https://github.com/ankidroid/Anki-Android/blob/master/AnkiDroid/src/main/res/values/theme_black.xml or https://github.com/ankidroid/Anki-Android/blob/master/AnkiDroid/src/main/res/values/theme_dark.xml - but I could be totally wrong there

Good luck :-)

Kartikkman commented 5 years ago

Hii @mikehardy , I have setup Git Stuff ( forking , .. cloning & all ) . Code is now set up & is running on my phone .

Kartikkman commented 5 years ago

Hey @mikehardy , Issue is a simple Fix . Setting the " navigationBarColor" Property in [https://github.com/ankidroid/AnkiAndroid/blob/master/AnkiDroid/src/main/res/values/theme_black.xml]()

<item name="android:navigationBarColor">@color/black</item>

Screen Shots :

Before :

ScreenShot_Before

After :

ScrrenShot_After

But This was introduced after API Level 21 , so won't support on KitKat .

Another way , I think we can proceed with this is use the method which works on API level less than 21 . Below is the method :

window.setNavigationBarColor(@ColorInt int color)

We can call this method when "Nighmode" Switch is activated & dectivated . Setting Navigation Bar to :

Black Color when Activated White Color when Deactivated

@mikehardy , Your views on it ??

Kartik Sikka

mikehardy commented 5 years ago

Hey cool - that was quick. I'd go with a solution that was clean first - and the XML is overlayed for API, so you can change it here and it won't crash old phones: https://github.com/ankidroid/Anki-Android/blob/master/AnkiDroid/src/main/res/values-v21/styles.xml

Then for APIs less than 21...Google tells me that's a total of 35,000 users out of 1,200,000, with about 10,000 about to be below minimum support shortly, and the rest declining at 15,000 a year. I'd save the effort as doing API-specific code things isn't great and there are more important things if you've got a dev environment set up. For instance #5097 this one is hurting a lot of people and we haven't even reproduced it yet :-(

So if the style overlay from values-v21 worked I'd PR that and go with it

Kartikkman commented 5 years ago

Hey @mikehardy

I have made changes in https://github.com/ankidroid/Anki-Android/blob/master/AnkiDroid/src/main/res/values-v21/styles.xml

It is Working . Below are the screenshots :

Working

But there is flaw with above thing , It doesn't work while we are in Settings Window ( I think we call it as Preference Fragment ??) . Below is the Screenshot :

NoChangeInSettingsWindow

@mikehardy , Are we Okay with this ?? Or Should I look for the solution of above sub problem as well ??

About the Issue #5097 , I will be surely working on it ( If I can Understand that , This is all new to me :) , Can You Guide me for that ? ) .

Thank You

mikehardy commented 5 years ago

Part of the fun is sort of digging in and searching around on stackoverflow when you have no idea what is going on ;-). The things you learn...

For the preferences not respecting navigationBarColor - that's really weird. It seems incomplete without that - digging in on that would be nice - but if there is no good answer then at least it is rarely used vs the navigationBar in the main theme

For #5097 - my first step would be to try to find a report that was as close to an emulator as I had (same Android version) and configure it for their language, clean everything out, do an install and see if you can reproduce it based on the steps the users report. We haven't found out what's causing it at all.

mikehardy commented 5 years ago

I see the "Layout Editor" tool in Android Studio, that I'd never played with before. Maybe that will allow inspection of the navigation bar while your emulator is showing the odd preferences behavior? I literally just tried it the first time out of curiosity so I have no good specifics on it, and the Pixel 3 / API29 emulator I tried doesn't exhibit the behavior so all I have is the idea of trying the tool. Hope it helps?

Kartikkman commented 5 years ago

Hii @mikehardy , Sorry for Replying Late ( was busy with Office Stuff ) . Well for the above I need to learn " Layout Inspector " , I am trying with it !!!

There is one more thing that I wanted to ask is that : " Is the theme attribute defined for Preferences ?? I on my effort couldn't find that .

I could see see the Different separate files for The Preferences but not the combined one where I could verify the above .

Although here is the What I captured from Layout Inspector ( I couldn't understand but maybe you can? )

LayoutInspector_Correct

LayoutInspector_Wrong

@mikehardy ??

Kartikkman commented 5 years ago

@mikehardy Are we still working on it ??

mikehardy commented 5 years ago

sorry - if you edit a previous comment instead of making a new one, it does not trigger any sort of notification for me, so I was simply unaware you were looking for feedback

Seems that we don't have a good answer for preferences - I don't have the answers to your questions, I'm not great at Android layout, sorry

If there's not 100% answer it might be time to at least make a pull request - have you ever made one?

Just in case you haven't (ignore me if you have) - typically the work you've been doing would be on a git branch (to leave master clean) - checked out like so git checkout -b 'issue5409/fix-status-bar-dark-mode' or whatever you want to call the branch

Once you have it working well locally, make sure ./gradlew jacocoUnitTestReport still works in your local environment, then git push back up to your branch on your fork on github

At that point in the github.com web interface it will probably prompt you to make a pull request, do that and just follow the template. Should be easy (it's never easy the first time)? :-)

Kartikkman commented 5 years ago

Hii @mikehardy , I tried to create Pull Request , Can You Please Check it out ??

One More Thing :

This gradlew Test stuck at 99 % . gradlew Test

@mikehardy , I am thankful for all your support , I am learning this all & will do my best to Deliver back to you .

Can you assign me next issue ( for Beginners ) , you may suggest me ??

Thanks Again !!!

mikehardy commented 5 years ago

Huh, that's strange regarding the 99% completion. Honestly I haven't tried it on windows ever, I wonder if there is an operating system specific error in the tests somewhere? I run them on ubuntu and macOS. I'll have to check it.

The PR looks fine though - and I don't see how it could do anything that would cause a problem for the tests. I'm sure it will pass our continuous integration tests

Thanks for the effort!

I think #5097 is our highest priority issue by far. It may not be easy but just trying to reproduce it at least is really valuable effort.

As for another good first issue I think #4968 is a good one - it gets into the Java code but is not a difficult change so it makes for a good start

pablode commented 5 years ago

@Kartikkman to answer your question, the preferences theme is defined in the App Manifest.

<activity
    android:name="com.ichi2.anki.Preferences"
    android:exported="false"
    android:configChanges="keyboardHidden|orientation|locale|screenSize"
    android:label="@string/preferences_title"
    android:theme="@style/LegacyActionBarLight"/>

However, changing it won't fix the issue, because it gets swapped at runtime:

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        Themes.setThemeLegacy(this);
        // ...
    }

If you follow the code trail you will find that neither Theme_Dark_Compat nor Theme_Black_Compat are being used. Instead, LegacyActionBarDark or LegacyActionBarBlack is chosen. You have to add the navigationBarColor attribute to them, too.

mikehardy commented 5 years ago

@pablode nice detective work - this could be a quick win with just another tiny XML resource tweak @Kartikkman

Kartikkman commented 5 years ago

Hey @pablode that's amazing , This is something I could never have come upto . I am making the change to Code Now , ( Would you like to share your Secret Recipe: How you find out this ) ?

pablode commented 5 years ago

There is no secret recipe, it's just years of accumulated knowledge and some basic deductive reasoning. Invest some time and eventually, you will get there, too!

mikehardy commented 5 years ago

Sounds about right re: experience. It just takes time. I'm on maybe 1.5 years of Android work and I couldn't find the preference theme either when I first trawled through. I must need more time :-)

github-actions[bot] commented 4 years ago

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

david-allison commented 4 years ago

I believe this is closed in #5417, please reopen if this is not the case.