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

fix: card title color #1675

Closed

Conversation

Ornanovitch
Copy link

@Ornanovitch Ornanovitch commented Sep 16, 2024

On master, the colorEditText color is unreadable since some updates of nextcloud/android-common (here I guess)

This commit makes the card title readable again with a more used and (stable?) function, colorTextView with the param ColorRole.ON_SURFACE

Before After
title-before title-after

the color was unreadable since some updates of nextcloud/android-common
this commit make the card title readable again with a more used and
(stable?) function
@stefan-niedermann
Copy link
Owner

Thank you for the contribution! Shouldn't this fixed upstream in the android-common library? I didn't look at the code yet, but I think the colorEditText method handles more stuff than the colorTextView method (cursor color, the color of the bottom line etc...)?

Copy link
Author

@Ornanovitch Ornanovitch left a comment

Choose a reason for hiding this comment

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

You're certainly right, I'll try to better understand the changes in android-common bu we should first test before/after the recent versions of the library. I'll try to get some time to do that.

I'll also try to just remove those lines, because this is the unique case of explicit usage of colorEditText in the whole app, while the other EditText fields are working (labels, assing, due date).

In any case upstream should be investigated but I'm just starting to learn all of this

@@ -17,6 +17,7 @@
import com.google.android.material.dialog.MaterialAlertDialogBuilder;
import com.google.android.material.tabs.TabLayout;
import com.google.android.material.tabs.TabLayoutMediator;
import com.nextcloud.android.common.ui.theme.utils.ColorRole;
Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
import com.nextcloud.android.common.ui.theme.utils.ColorRole;

@@ -289,7 +290,7 @@ private void applyTheme(int color) {

utils.platform.themeStatusBar(this);
utils.material.themeToolbar(binding.toolbar);
utils.platform.colorEditText(binding.title);
utils.platform.colorTextView(binding.title, ColorRole.ON_SURFACE);
Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
utils.platform.colorTextView(binding.title, ColorRole.ON_SURFACE);

@Ornanovitch
Copy link
Author

Oh! Could it be that?

nextcloud/android-common@50e641e#diff-f2305b95fc7795d7fc2ff8c309ead47432f28737b0fb5336b690876a48623572R620

- editText.setTextColor(scheme.onSurface)
+ editText.setTextColor(dynamicColor.surface().getArgb(scheme)) // -> .onSurface becoming .surface

Ornanovitch referenced this pull request in nextcloud/android-common Sep 17, 2024
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@Ornanovitch
Copy link
Author

superseded by nextcloud/android-common#502

Let's close this until we can see the library changes

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.

2 participants