Skip to content

Add theming logic for share link icons #6400

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.

Sign up for GitHub

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

Merged
tobiasKaminsky merged 1 commit into from
Jul 2, 2020
Merged

Add theming logic for share link icons #6400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions src/main/java/com/owncloud/android/ui/adapter/PublicShareViewHolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@
package com.owncloud.android.ui.adapter;

import android.content.Context;
import android.graphics.PorterDuff;
import android.text.TextUtils;
import android.view.View;

import com.owncloud.android.R;
import com.owncloud.android.databinding.FileDetailsSharePublicLinkItemBinding;
import com.owncloud.android.lib.resources.shares.OCShare;
import com.owncloud.android.utils.ThemeUtils;

import androidx.annotation.NonNull;
import androidx.recyclerview.widget.RecyclerView;
Expand All @@ -52,15 +51,8 @@ public void bind(OCShare publicShare, PublicShareInterface listener) {
if (!TextUtils.isEmpty(publicShare.getLabel())) {
binding.publicShareLabel.setText(publicShare.getLabel());
}
binding.copyInternalLinkIcon
.getBackground()
.setColorFilter(context.getResources().getColor(R.color.primary_button_background_color),
PorterDuff.Mode.SRC_IN);
binding.copyInternalLinkIcon
.getDrawable()
.mutate()
.setColorFilter(context.getResources().getColor(R.color.black),
PorterDuff.Mode.SRC_IN);

ThemeUtils.colorIconImageViewWithBackground(binding.copyInternalLinkIcon, context);

binding.shareLinkCopyIcon.setOnClickListener(v -> listener.copyLink(publicShare));

Expand Down
33 changes: 25 additions & 8 deletions src/main/java/com/owncloud/android/utils/ThemeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,15 +688,9 @@ public static String colorToHexString(int color) {
public static void colorFloatingActionButton(FloatingActionButton button, @DrawableRes int drawable,
Context context) {
int primaryColor = ThemeUtils.primaryColor(null, true, false, context);
colorFloatingActionButton(button, context, primaryColor);

if (Color.BLACK == primaryColor) {
button.setImageDrawable(ThemeUtils.tintDrawable(drawable, Color.WHITE));
} else if (Color.WHITE == primaryColor) {
button.setImageDrawable(ThemeUtils.tintDrawable(drawable, Color.BLACK));
} else {
button.setImageDrawable(ThemeUtils.tintDrawable(drawable, ThemeUtils.fontColor(context, false)));
}
colorFloatingActionButton(button, context, primaryColor);
button.setImageDrawable(ThemeUtils.tintDrawable(drawable, getColorForPrimary(primaryColor, context)));
}

public static void colorFloatingActionButton(FloatingActionButton button, Context context) {
Expand All @@ -712,6 +706,29 @@ public static void colorFloatingActionButton(FloatingActionButton button, int ba
button.setRippleColor(rippleColor);
}

public static void colorIconImageViewWithBackground(ImageView imageView, Context context) {
int primaryColor = ThemeUtils.primaryColor(null, true, false, context);

imageView.getBackground().setColorFilter(primaryColor, PorterDuff.Mode.SRC_IN);
imageView.getDrawable().mutate().setColorFilter(getColorForPrimary(primaryColor, context),
PorterDuff.Mode.SRC_IN);
}

/**
* returns a primary color matching color for texts/icons on top of a primary-colored element (like buttons).
*
* @param primaryColor the primary color
*/
private static int getColorForPrimary(int primaryColor, Context context) {
if (Color.BLACK == primaryColor) {
Copy link
Member

@tobiasKaminsky tobiasKaminsky Jul 1, 2020

Choose a reason for hiding this comment

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

Is this obeying dark mode?

Copy link
Member Author

@AndyScherzinger AndyScherzinger Jul 1, 2020

Choose a reason for hiding this comment

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

Not at this very moment, we just do this based on the primary color for edge cases and else use the server's text-color value. See also my other comment.

return Color.WHITE;
} else if (Color.WHITE == primaryColor) {
return Color.BLACK;
} else {
return ThemeUtils.fontColor(context, false);
Copy link
Member

@tobiasKaminsky tobiasKaminsky Jul 1, 2020

Choose a reason for hiding this comment

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

Isn't this also a good idea to have this computed on server? font-color-light and font-color dark?

Copy link
Member Author

@AndyScherzinger AndyScherzinger Jul 1, 2020

Choose a reason for hiding this comment

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

Don't know, but @nickvergessen might be able to shed some light on the font-color calculations. I'd hope that with a fallback-ing background color working for dark/light the text-color of a button doesn't have any contrast issues anymore. So we would rather have to discuss if primary button texts would have to be white/black or if we just use white in both theme variants.

So also pinging @jancborchardt about this. ATM we don't have text-color variant from the server (also with element-color reference)

Copy link
Member

@tobiasKaminsky tobiasKaminsky Jul 2, 2020

Choose a reason for hiding this comment

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

Fine for me to have this in next PR 👍

}
}

private static OCCapability getCapability(Context context) {
return getCapability(null, context);
}
Expand Down