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): Align theming closer to files app #1752

Merged
merged 6 commits into from
Apr 11, 2023

Conversation

stefan-niedermann
Copy link
Member

@stefan-niedermann stefan-niedermann commented Apr 7, 2023

Follow-Up to #1680

Open issues:

  • SettingsActivity: Back arrow is not perfectly aligned with icons. I propose to just keep it as it is for now.
  • The toolbar icons do have a different position in the main activity compared to each other activity - because of placing it inside of the rounded card. This behavior will be the same when we switch to the material SearchBar
  • ActionMode could need some more love

Out of scope:

  • Switching to an actual material SearchBar for the main view
_ _
image image

Would love to get some feedback @AndyScherzinger

Copy link
Member

@AndyScherzinger AndyScherzinger left a comment

Choose a reason for hiding this comment

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

looks good to me @stefan-niedermann 👍

@AndyScherzinger
Copy link
Member

Minor detail, the 3-dot overflow menu icons should be tinted the same way the other trailing icons are tinted.

see:

https://github.com/nextcloud/android-common/blob/08db28c1a4ca825c82ddf56747d7892c719550a7/ui/src/main/java/com/nextcloud/android/common/ui/theme/utils/MaterialViewThemeUtils.kt#L58-L62

@AndyScherzinger
Copy link
Member

Minor detail, the 3-dot overflow menu icons should be tinted the same way the other trailing icons are tinted.

Well, it is also like that in the talk app, so maybe keep it as is for the moment 👍

@stefan-niedermann
Copy link
Member Author

stefan-niedermann commented Apr 9, 2023

the 3-dot overflow menu icons should be tinted the same way the other trailing icons are tinted.

Maybe themeToolbar should call colorToolbarOverflowIcon implicitly? It would fix also the Talk app then and would at least match my persobal expectations...

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Apr 9, 2023

I now fixed in for talk explicitly but yeah, makes sense to call it within the toolbar theming logic 👍

@AndyScherzinger
Copy link
Member

android-common 0.8.0 ships a fix for theming the trailing icons (overflow icon) when theming the toolbar. 👍

@stefan-niedermann stefan-niedermann marked this pull request as ready for review April 11, 2023 06:38
@stefan-niedermann
Copy link
Member Author

👍 Ready to merge from my side

Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
Fix navigation view background

Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
Follow-Up to #1680

Signed-off-by: Stefan Niedermann <[email protected]>
@AndyScherzinger AndyScherzinger force-pushed the align-theming-to-files-app branch from b554849 to fc4ed90 Compare April 11, 2023 08:04
@AndyScherzinger AndyScherzinger added this to the 4.1.0 milestone Apr 11, 2023
@AndyScherzinger AndyScherzinger merged commit 01fc67a into main Apr 11, 2023
@AndyScherzinger AndyScherzinger deleted the align-theming-to-files-app branch April 11, 2023 09:30
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.

2 participants