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

feat(theming): Use nextcloud-common library for theming UI elements (#1648) #1680

Merged
merged 6 commits into from
Feb 16, 2023

Conversation

stefan-niedermann
Copy link
Member

@stefan-niedermann stefan-niedermann commented Feb 12, 2023

This is an intermediate step to an alignment with the Material Files and Talk app.

Most important class: BrandingUtil - is more or less what you know as ViewThemeUtils in the Files app.

Something more you should know: I created also a class NotesViewThemeUtils as a place to put app specific styling stuff until they are available within the android-common library. It will probably fail because of the "No more Java" GitHub action you put in place 😉 So either we need to convert it to Kotlin or you make an exception for intermediate steps 🤷‍♂️ .

  • The obvious stuff looks good (FloatingActionButtons, EditTexts, SwitchPreferences and so on) and is already done via android-common
  • Other parts like Toolbars might take some more time to iron out.

If you have own plans or think this goes into the wrong direction, please let me know.

👁️ Screenshots

Only made with base color #0082C9 for time reasons, but the other colors look as expected and aligned with the Files app, too.

Light Dark
grafik grafik
grafik grafik
grafik grafik
grafik grafik
grafik grafik

Signed-off-by: Stefan Niedermann <[email protected]>
... and rely on PlatformThemeUtil.isDarkMode in stead.

Signed-off-by: Stefan Niedermann <[email protected]>
@stefan-niedermann
Copy link
Member Author

So I will stop here and give you some time to for a review - sorry, it already grew larger than I originally planned...

@AndyScherzinger
Copy link
Member

The screenshots look good to me 👍

@tobiasKaminsky
Copy link
Member

Fine by me.
Please check with Alvaro when to merge, as he is also currently working quite a lot on Notes.
Might be that it is wise to postpone this, or merge as early as possible to prevent conflicts.

* Only add the needed module rather than the entire lib
* Use ColorRole where possible in favor of querying the wanted color from the Scheme directly
* Use util for theming SwipeRefreshLayout

Signed-off-by: Stefan Niedermann <[email protected]>
…vor of AndroidViewThemeUtils#highlightText

Signed-off-by: Stefan Niedermann <[email protected]>
Copy link
Member

@AlvaroBrey AlvaroBrey left a comment

Choose a reason for hiding this comment

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

Thank you very much @stefan-niedermann !

@AlvaroBrey AlvaroBrey merged commit 1f1cdc0 into main Feb 16, 2023
@AlvaroBrey AlvaroBrey deleted the theme-alignment branch February 16, 2023 12:26
@AlvaroBrey AlvaroBrey added this to the 4.0.0 milestone Feb 16, 2023
@stefan-niedermann
Copy link
Member Author

A bit too late, but I just noticed, that this will unresolve #769 - see also nextcloud/android-common#79

It is not a big deal yet, because the excerpts are quite small.

I want to use the theming in the nextcloud-commons lib too, so the in note search aligns with the theme, and there it would be quite breaking...

stefan-niedermann added a commit that referenced this pull request Apr 7, 2023
Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
stefan-niedermann added a commit that referenced this pull request Apr 7, 2023
Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
stefan-niedermann added a commit that referenced this pull request Apr 7, 2023
Fix navigation view background

Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
stefan-niedermann added a commit that referenced this pull request Apr 11, 2023
Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
AndyScherzinger pushed a commit that referenced this pull request Apr 11, 2023
Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
AndyScherzinger pushed a commit that referenced this pull request Apr 11, 2023
Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
AndyScherzinger pushed a commit that referenced this pull request Apr 11, 2023
Fix navigation view background

Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
AndyScherzinger pushed a commit that referenced this pull request Apr 11, 2023
Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
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.

4 participants