Skip to content

Conversation

@AndyScherzinger
Copy link
Member

@AndyScherzinger AndyScherzinger commented Jun 22, 2020

Resolves #6323

edge case improvements (dark/light):

  • active drawer menu item contrast
  • primary button coloring

OPEN

  • light-theme+white for MaterialButtons
  • dark-theme+white for MaterialButtons
  • FAB coloring black/white extreme scenarios for dark/light themes
  • probably any MaterialButton (maybe add a theming method for buttons handling all edge cases and securing consistent theming, to be done for standard and borderless buttons)
  • replace use of elementColor(...) with primaryColor(...) and drop method

cc @dan0xii for collaboration

Testing

Writing tests is very important. Please try to write some tests for your PR.
If you need help, please do not hesitate to ask in this PR for help.

unit tests
instrumented tests
UI tests

  • Tests written, or not not needed

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Jun 23, 2020

Button coloring examples

#ffffff server primary, client light theme #ffffff server primary, client dark theme
device-2020-06-23-113437 device-2020-06-23-105122
#000000 server primary, client light theme #000000 server primary, client dark theme
device-2020-06-23-101138 device-2020-06-23-102429

@jancborchardt see screenshots above this is the edge case handling I created for light/dark scenarios and primary buttons. Would this be fine for you?

Default Nextcloud blue

#0082C9 server primary, client light theme #0082C9 server primary, client dark theme
Screenshot_20200623-121255_Nextcloud Screenshot_20200623-121237_Nextcloud

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks veeeery nice! :)

@nextcloud-android-bot
Copy link
Collaborator

master-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

@nextcloud-android-bot
Copy link
Collaborator

@AndyScherzinger AndyScherzinger force-pushed the enh/darkThemeOptimizations branch from acd018f to 2f7d8c2 Compare June 24, 2020 20:09
@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@AndyScherzinger AndyScherzinger force-pushed the enh/darkThemeOptimizations branch from 2f7d8c2 to d6fdb67 Compare June 24, 2020 21:22
@nextcloud-android-bot
Copy link
Collaborator

stable-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

@nextcloud-android-bot
Copy link
Collaborator

master-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

stable-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

stable-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

@nextcloud-android-bot
Copy link
Collaborator

@AndyScherzinger AndyScherzinger force-pushed the enh/darkThemeOptimizations branch from c377d33 to a856513 Compare June 25, 2020 11:01
@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@AndyScherzinger AndyScherzinger force-pushed the enh/darkThemeOptimizations branch from 80f15ce to 9644e9c Compare June 25, 2020 11:41
@AndyScherzinger AndyScherzinger removed their assignment Jun 25, 2020
@AndyScherzinger AndyScherzinger force-pushed the enh/darkThemeOptimizations branch from 0efadad to 9b7cdef Compare June 25, 2020 21:53
@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/14690.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

Codacy

391

Lint

TypemasterPR
Warnings8585
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings61
Internationalization Warnings9
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings74
Security Warnings44
Dodgy code Warnings111
Total339

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings62
Internationalization Warnings9
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings74
Security Warnings44
Dodgy code Warnings111
Total340

@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/14694.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

@nextcloud-android-bot
Copy link
Collaborator

Codacy

391

Lint

TypemasterPR
Warnings8585
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings61
Internationalization Warnings9
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings74
Security Warnings44
Dodgy code Warnings111
Total339

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings62
Internationalization Warnings9
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings74
Security Warnings44
Dodgy code Warnings111
Total340

@AndyScherzinger AndyScherzinger force-pushed the enh/darkThemeOptimizations branch from 145c296 to 5f73a5e Compare June 26, 2020 13:16
@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/14723.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

Codacy

391

Lint

TypemasterPR
Warnings8585
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings61
Internationalization Warnings9
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings74
Security Warnings44
Dodgy code Warnings111
Total339

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings62
Internationalization Warnings9
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings74
Security Warnings44
Dodgy code Warnings111
Total340

Copy link
Collaborator

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- src/main/java/com/owncloud/android/utils/ThemeUtils.java  1
         

See the complete overview on Codacy

@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/14724.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

Codacy

391

Lint

TypemasterPR
Warnings8585
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings61
Internationalization Warnings9
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings74
Security Warnings44
Dodgy code Warnings111
Total339

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings27
Correctness Warnings62
Internationalization Warnings9
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings74
Security Warnings44
Dodgy code Warnings111
Total340

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Black primary color on Android dark theme

6 participants