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

Use DayNight themes #1547

Merged
merged 17 commits into from Oct 2, 2019
Merged

Use DayNight themes #1547

merged 17 commits into from Oct 2, 2019

Conversation

mueller-ma
Copy link
Member

@mueller-ma mueller-ma commented Sep 21, 2019

Fixes #873
Fixes #1058
Closes #1431

TODO:

  • Fix crash when opening About on Android 5
  • Fix section switch widgets

@mueller-ma mueller-ma added the enhancement Indicates new feature requests label Sep 21, 2019
@mueller-ma
Copy link
Member Author

Copy link
Contributor

@maniac103 maniac103 left a comment

Choose a reason for hiding this comment

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

Section switch height is due to the Widget.MaterialComponents.CompoundButton.RadioButton style having a android:minHeight of 48dp set. We can/should override this in widgetlist_sectionswitchitem_button.xml.

I'm still not really convinced that the accent color selection is a useful addition (for the same reasons as mentioned earlier, but it looks like it's important to you?

@mueller-ma
Copy link
Member Author

We can/should override this in widgetlist_sectionswitchitem_button.xml.

👍

I'm still not really convinced that the accent color selection is a useful addition (for the same reasons as mentioned earlier, but it looks like it's important to you?

It's possible to change the accent color with the current implementation and it's also possible to the Basic UI and many other UIs. Thus I don't want to remove this feature from the app.

@maniac103
Copy link
Contributor

It's possible to change the accent color with the current implementation

Yes, but only between a set of color schemes related to openHAB.

and it's also possible to the Basic UI and many other UIs.

Where can one change the Basic UI color scheme? I've only ever seen such settings in some Android apps not affiliated to a certain project or company so far.

Thus I don't want to remove this feature from the app.

You want to add this feature; that's not the same thing as not removing it. One currently can't select arbitrary color schemes.

@mueller-ma
Copy link
Member Author

Where can one change the Basic UI color scheme? I've only ever seen such settings in some Android apps not affiliated to a certain project or company so far.

I've misread a PR for theming Basic UI: eclipse-archived/smarthome#4137
The colors didn't make it through review, only bright and dark mode. I reduced the accent colors to orange, blue (like Basic UI) and grey (like the old grey theme).

@maniac103
Copy link
Contributor

I've misread a PR for theming Basic UI: eclipse-archived/smarthome#4137
The colors didn't make it through review, only bright and dark mode.

Heh, I see Kai shared my concerns in https://github.com/eclipse/smarthome/pull/4137/files#r137015539 ;-)

I reduced the accent colors to orange, blue (like Basic UI) and grey (like the old grey theme).

Nice. Now we just need to restore the pink accent color in the blue theme to make it consistent to BasicUI and we're set, I think.

Copy link
Contributor

@maniac103 maniac103 left a comment

Choose a reason for hiding this comment

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

Found a few additional small things.

I also wonder whether we shouldn't go to minSdkVersion 21 ... doing so allows using color resources or attributes in vector drawables, which means we could drop all -night drawables.

@mueller-ma
Copy link
Member Author

I also wonder whether we shouldn't go to minSdkVersion 21

I'd like to keep this for now.

@mueller-ma mueller-ma changed the title Use DayNight themes and add more accent colors Use DayNight themes Sep 27, 2019
@maniac103
Copy link
Contributor

maniac103 commented Sep 28, 2019

Hmm, opening settings has a delay of at least half a second on a fairly fast device (OnePlus 6). This seems too much for my taste, and I don't think it's always been like that, right?

Edit: Looks like this is unrelated -> #1563

@maniac103
Copy link
Contributor

maniac103 commented Sep 30, 2019

Got 2 additional optimization commits to pull: https://github.com/maniac103/openhab.android/commits/dark-mode ... the 'avoid recreation' one was inspired by advice in https://medium.com/androiddevelopers/appcompat-v23-2-daynight-d10f90c83e94

@mueller-ma mueller-ma force-pushed the dark-mode branch 2 times, most recently from c2f5334 to 939ef16 Compare September 30, 2019 14:46
mueller-ma and others added 12 commits October 2, 2019 08:12
Fixes openhab#873
Fixes openhab#1058
Closes openhab#1431

Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: Danny Baumann <dannybaumann@web.de>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
mueller-ma and others added 4 commits October 2, 2019 08:12
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: Danny Baumann <dannybaumann@web.de>
In particular, avoid recreating activities if possible.

Signed-off-by: Danny Baumann <dannybaumann@web.de>
The "Select" button in the accent color picker in a dark theme and
grey as accent color is hardly visible.

Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Info ssl client cert" tooltip has bright background in dark themes Add custom theme
2 participants