ankidroid / Anki-Android

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

More consistent dark mode experience #12473

Open user1823 opened 2 years ago

user1823 commented 2 years ago

In the latest alpha releases, the screens that have inherited code from Desktop Anki (e.g. the Statistics page) have gray background in Night Mode while the rest of AnkiDroid has a black background.

In my opinion, these new pages should also have a black background for a more consistent experience.

mikehardy commented 2 years ago

Interesting - this is going to cut across the repos - I think there was some script injection in our use of the desktop HTML/JS/CSS, I wonder if we have control over the theme in those? @krmanik are we able to change the colors of the desktop HTML/JS/CSS ?

krmanik commented 2 years ago

AnkiDroid provides four sets of theme, Day theme - (Light, Plain), Night theme - (Black, Dark). In AnkiDesktop only two sets of theme available day and night. In desktop code we have to add css rules in base.css. It can be done. For e.g. in card-info page, we have to add something like ankidroid-light, ankidroid-plain, ankidroid-dark and ankidroid-black rules. https://github.com/ankitects/anki/blob/main/ts/card-info/card-info-base.scss

Other solution may be css injected before loading of the webpages using webview.loadUrl. It is also possible, but above approach is better.

BrayanDSO commented 2 years ago

Oh. Injecting CSS is completely possible. I've tried before and it worked just fine (see screenshots here https://github.com/ankidroid/Anki-Android/issues/6772#issuecomment-1208395273), but never got the time to think on a implementation I really liked.

krmanik commented 2 years ago

I think we should consider adding css files in assets dir and inject using webview. It will be helpful when small changes does not require to build the backend again then load those pages. The css will be specific to AnkiDroid only.

BrayanDSO commented 2 years ago

Fine by me. Will the CSS files be generated dynamically? So we are always up to date with the colors used by the themes.

Go with the idea below 👇

Another thought: maybe there is some say to create a test to see if the CSS is valid

mikehardy commented 2 years ago

With no intention to be prescriptive (my opinion: literally whatever seems good to you two, you are definitely the experts here), even if the CSS could not be generated from the themes xml (or even if it could :thinking: ) a comment in the themes that changes must be made in concert with the CSS would be sufficient for me. We don't change them that much, and if there's a <!-- you must keep foo.css in sync with this by changing there too --> a reviewer knows what to look for a dev knows what to do

BrayanDSO commented 2 years ago

I like your idea more. The simple is always the best answer

dae commented 2 years ago

Anki uses css vars which make styling changes fairly easy, but there have been tweaks to them in the current development code, so I'd recommend holding off on pursuing this until 2.1.55 is out or AnkiDroid has updated to a current snapshot

BrayanDSO commented 2 years ago

Almost always easy. IIRC, some colors like the "Import" button on the CSV importer screen didn't have a var associated. But a PR changing this on upstream should be fairly easy if this still persists

mikehardy commented 2 years ago

I just set a "watch" for releases on ankitects/anki ;-) - will resync backend then

github-actions[bot] commented 2 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

user1823 commented 2 years ago

Waiting for the next release on ankitects/anki

github-actions[bot] commented 1 year 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

dae commented 1 year ago

@BrayanDSO still an issue?

BrayanDSO commented 1 year ago

Yes. It is

david-allison commented 11 months ago

@BrayanDSO fixed in e04d9be7cdab47044057ddd9f07ef634bb99e382 ?

user1823 commented 11 months ago

Partially

For example, the Stats page looks like the following in alpha 10:

The Card Info looks like the following:

BrayanDSO commented 11 months ago

@BrayanDSO fixed in https://github.com/ankidroid/Anki-Android/commit/e04d9be7cdab47044057ddd9f07ef634bb99e382 ?

IMO, yes. Making the “surface” background (the grey rounded rectangles in the Statistics print @user1823 linked) black in the black theme is IMHO poor design, because that means that there shouldn’t be any contrast between the blocks in a page.

"But that is how the black theme looks like" -> that means there is a problem with the black theme, not with the Anki pages themselves.

user1823 commented 11 months ago

Making the “surface” background black in the black theme is IMHO poor design

I agree, but I think that they can still be made slightly darker.

However, I am not an expert in this field. So, I leave the final decision to you.

BrayanDSO commented 11 months ago

I agree, but I think that they can still be made slightly darker.

I agree. It is something that I plan to do in my #13878 work.

But to avoid having this issue opened too long, since "being more consistent" is something subjective and we always "can be more consistent", I would like to have some criterium to close this, which IMO was having the main background using the same color, and actually what you originally put in the issue description.

user1823 commented 11 months ago

I would like to have some criterium to close this

I think fixing the Stats page (and Deck Options page) should be the criterion to close this.

Currently, these pages are almost the same as before because the main background is hardly visible in these pages.

I would like to separate this issue from https://github.com/ankidroid/Anki-Android/issues/13878 because updating the whole UI would take a lot of time but the current issue is relatively much easier to resolve.

david-allison commented 9 months ago

@user1823 we've done some work here. Is there anything still remaining?

user1823 commented 9 months ago

In my opinion, the surface background (the grey rounded rectangles in the Stats page, Deck Options page, etc.) should be darker than they currently are (though not completely black) when in the black theme.