Skip to content
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

Add a dark/night mode #1306

Merged
merged 30 commits into from
Mar 22, 2019
Merged

Add a dark/night mode #1306

merged 30 commits into from
Mar 22, 2019

Conversation

ENT8R
Copy link
Contributor

@ENT8R ENT8R commented Dec 30, 2018

As said in #942 (comment) I put some more work β›‘ into this branch and it is not a setting πŸ‘¨β€πŸ”§ for selecting a mapstyle anymore but rather a setting for selecting a theme 🎭 which implicates an automatic selection of a mapstyle πŸ—Ί.

  • the setting consists of 3 (or 4) parts
  1. Auto (uses the dark theme from 22:00 to 06:00 and if the (or one of the last) location is known, it calculates automatically the sunrise/sunset times and changes the theme accordingly
  2. Light (the normal theme which is also used currently)
  3. Dark (the dark theme which changes the mapstyle to the dark one and also changes the app colors to use as much black as possible πŸ˜„)
  4. System default (only works on Android Pie and higher and uses a system-wide setting as announced here. I will try to set up a new emulator with Android Pie and test it during the next days...)

I also made some other smaller changes which I noticed while testing:

  • changed some icons which are grey to be white in night mode for better contrast (e.g. some "religious" icons)
  • added a navigation bar color which also changes when the theme changes (can be seen on some of my screenshots)

And finally, here are some images of the new dark mode:


Fixes #198 and fixes #673

@rugk
Copy link
Contributor

rugk commented Dec 31, 2018

(1) Hmm, why do you use so much transparency there (e.g. for buildings?) Can't you just use solid (darker) colors?
It looks a little futuristic, as far as I see.

(2) Also it really looks like a "night theme". It is e.g. not really suited for OLED-devices to be used at day times, as it does not have a string contrast.
But this is okay, there may even be separate themes in the future for this.

(3) Considering this a "night theme", what really sparks my eye in the second screenshot are the very light & bright icons. The do not at all fit the (map) theme/background and thus not only look displayed, but also strain your eyes if you are looking at this in the night.
You obviously cannot re-design them all, but possibly you can just apply some filters, turning down the brightness and/or saturation or so?

@ENT8R
Copy link
Contributor Author

ENT8R commented Dec 31, 2018

(1) Hmm, why do you use so much transparency there (e.g. for buildings?) Can't you just use solid (darker) colors?

Well, that's probably a thing of the mapstyle (https://github.com/ENT8R/streetcomplete-mapstyle)...

@westnordost
Copy link
Member

westnordost commented Dec 31, 2018

It is pretty cool that you do this. Creating a night-theme has been on my list since I redid the quest UI. I have not looked at the PR yet because I currently do not have so much time, just a few things that come to my mind regarding this:

  • Are you basing the work on Android's built-in DayNightthemes? (I think this is important) You wrote something about that this is only available since Android Pie. But what about the appcompat libraries?
  • Switching automatically the theme based on the time and location (sunrise, sundown) is I think a built-in feature of Android, isn't it?
  • The screenshots look pretty good already. A few notes on that:
    • The contrast of the completely black speech bubbles is a bit too much when compared to the violet map background. Not sure though how this could be done better as I think dark grey would not look good.
    • The now black speechbubbles still have a black shadow, which makes it difficult to tell the border of the dialog (fourth picture). Maybe a white or an orange shadow? How do other (Google) apps do this?
    • The quest bubbles on the map stick out too much now. They could be made less sticking-out by giving the pins a different background than white. Black may be a problem because water is (currently) black in the mapstyle. This could be changed, of course, but a fitting color does not come to my mind right now - dark-turqoise? Overall, the current dark mapstyle is less colorful than the light mapstyle, maybe it doesn't have to be.
    • Wording: In the dialog, it shouldn't read "auto" but something like "automatically based on time" or something like that to make clearer whta "auto" does.

@ENT8R
Copy link
Contributor Author

ENT8R commented Dec 31, 2018

Are you basing the work on Android's built-in DayNightthemes? (I think this is important) You wrote something about that this is only available since Android Pie. But what about the appcompat libraries?

Yes, the whole thing is using the DayNight themes which are built-in. They are compatible back to API 14 or something like that using AppCompat. I only said that beginning from Android Pie, there is a system setting which allows the user to select a theme. All other apps should adapt this setting and theme their app accordingly

Switching automatically the theme based on the time and location (sunrise, sundown) is I think a built-in feature of Android, isn't it?

Yes, this is part of the DayNight theme feature πŸ‘

The now black speechbubbles still have a black shadow, which makes it difficult to tell the border of the dialog (fourth picture)

I also noticed this while testing. I will think about what might suit better here...

@westnordost
Copy link
Member

Is this already ready for review?

@ENT8R
Copy link
Contributor Author

ENT8R commented Jan 6, 2019

Mmhh.. I'd say not yet, I am still doing some tests. I will tell you if I think I'm done

app/src/main/assets/map_theme/scene.yaml Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/Prefs.java Outdated Show resolved Hide resolved
getActivity().recreate();
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary? The user can only change the setting in another activity.
(Also, this code should be in the activity)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I know, but the other activity (MainActivity) is not destroyed by switching to the settings activity, so the view is also not re-rendered when switching back from the settings activity to the main acitivity. But as it is stated here: https://blog.iamsuleiman.com/daynight-theme-android-tutorial-example/

Calling AppCompatDelegate.setDefaultNightMode(mode) must be done BEFORE calling super() in Activity onCreate(). Otherwise you must recreate() the Activity for the set theme to take effect

I added this small check in order to recreate the activity if necessary...

@westnordost
Copy link
Member

The conflicts that need to be resolved are from the appcompat -> androidx migration, replacing the imports with the new paths will solve it.

@ENT8R
Copy link
Contributor Author

ENT8R commented Feb 6, 2019

I finally found some time to continue to work on this branch...
I didn't change that much though: only some merge conflicts, your solution with the preference enumeration and some other smaller issues.
I also commented on one of your PR comments...

@westnordost
Copy link
Member

I checked it out and made some small adjustments in the hope that I can press it into the v10.0 release I want to do today, but I think I rather not rush it. Things that I would like to change still:

  • make the bottom navigation transparent on the main activity. I tried it out, actually, but couldn't get it to work with fitSystemWindows=true so that the map itself extends below the navigation bar but the buttons and quest form not. Perhaps it can't be done solely with the XML property. I find it not so important right now, so I will not look into it again until I redo the UI (for the top bar). But if you manage to implement it, that would make me very happy
  • I tried out the black quest pins. They do not look good because the arrow-part of the pin almost vanishes against the background. The icons are too bright in general to look good on a black background. So, I would rather keep the white pins or optionally play around with some other colors (turqoise?)
  • The speech bubbles and dialogs are too pitch-black. I think it would be better to have them in the default night-mode dark gray
  • maybe use another overlay-mode for the buildings because depending on how detailed a building is mapped, it is getting all the visual attention. Also maybe try another building color because the building color is (too) very similar to the selection color used in the app.

@westnordost
Copy link
Member

Also, I noticed a crash happening sometimes when I switch to the settings activity. The stack trace is:

java.lang.RuntimeException: Performing pause of activity that is not resumed: {de.westnordost.streetcomplete.debug/de.westnordost.streetcomplete.MainActivity}
        at android.app.ActivityThread.performPauseActivity(ActivityThread.java:3337)
        at android.app.ActivityThread.performPauseActivity(ActivityThread.java:3325)
        at android.app.ActivityThread.handlePauseActivity(ActivityThread.java:3300)
        at android.app.ActivityThread.access$1000(ActivityThread.java:156)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1362)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loop(Looper.java:211)
        at android.app.ActivityThread.main(ActivityThread.java:5389)
        at java.lang.reflect.Method.invoke(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:372)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1020)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:815)

It might have something to do with the Activity recreation:
https://stackoverflow.com/questions/10844112/runtimeexception-performing-pause-of-activity-that-is-not-resumed

@ENT8R
Copy link
Contributor Author

ENT8R commented Feb 12, 2019

make the bottom navigation transparent on the main activity

I don't really get what you mean... Do you want the quest form to be see-through?

But I fixed two more points of your last comment:

  • Switched to #303030 for dialogs and speech bubbles (seems to be the default of android). I didn't change the colour of the quest pin yet because this is another point.
  • Fixed the Performing pause of activity that is not resumed issue by delaying the recreation of the activity a little bit, so the activity has enough time to first recreate itself to get destroyed again afterwards. πŸ˜„

@westnordost
Copy link
Member

I don't really get what you mean... Do you want the quest form to be see-through?

No. In the screenshots you posted, at the very bottom, there is a triangle, a circle and a square on black / violet background. It is possible in Android to make that background transparent and let the view extend to under these buttons. Like for example in the launcher (the "desktop").

@ENT8R
Copy link
Contributor Author

ENT8R commented Feb 12, 2019

Thank you for clarification! I directly tried some things, but with no luck too 😞

@westnordost
Copy link
Member

I just watched https://www.youtube.com/watch?v=_mGDMVRO3iE , but I am still confused. Apparently, fitSystemWindows should be avoided but then, maybe the only way to set the right paddings would be programmatically.

@westnordost
Copy link
Member

On the other hand, neither Gmaps, nor maps.me, nor mapy.cz has a transparent navigation bar where the map is drawn below. So maybe it is not desirable for sc as well.

@westnordost
Copy link
Member

westnordost commented Feb 20, 2019

I noticed a few more things. Some I fixed right away, the ones that are still open:

  • tunnels have a weird color in dark mode
  • cycleway and sidewalk quests have an almost-white background color for the street side puzzle. I guess it should instead be a almost transparent grey so that it also looks good on dark mode
  • the create-note-marker is still black. Do you plan on experimenting there more?

@rugk
Copy link
Contributor

rugk commented Feb 21, 2019

BTW as far as I see, this does not completly fix #673, as #673 also requests to be able to switch to the "satellite map".

@westnordost
Copy link
Member

I don't know if my changes to the dark style were really positive. What do you think?

darkstyle

@rugk
Copy link
Contributor

rugk commented Feb 22, 2019

I like them. In contrast to before it now does not look so futuristic anymore.

But BTW, don't forget to push the changes to https://github.com/ENT8R/streetcomplete-mapstyle, too. πŸ˜„

@ENT8R
Copy link
Contributor Author

ENT8R commented Mar 3, 2019

the create-note-marker is still black. Do you plan on experimenting there more?

No. I probably just forgot to remove it.

I don't know if my changes to the dark style were really positive. What do you think?

I like it, but maybe the contrast between the building color and the normal landuse color is a little bit too low?

@westnordost westnordost merged commit 4d34dad into streetcomplete:master Mar 22, 2019
@ENT8R ENT8R deleted the map-setting branch March 22, 2019 15:41
@matkoniecz matkoniecz mentioned this pull request Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add setting for selecting map style Black/dark mode
3 participants