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

Feature/configurable table odd row alpha #354

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ntomasevic-vfy
Copy link

Odd rows of a table by default has set alpha channel (value is 22) but it should be configurable.

@noties
Copy link
Owner

noties commented Apr 16, 2021

Hello @ntomasevic-vfy ,

First of all, I appreciate your effort. But let me just ask - why would you need a special setting for alpha when you can specify color via tableOddRowBackgroundColor with alpha component directly?

@ntomasevic-vfy
Copy link
Author

Hi @noties ,
It is because your condition is not right, if you set tableOddRowBackgroundColor as completely transparent (@andorid:color/transparent) it won't be, you will have black color with opacity of TABLE_ODD_ROW_DEF_ALPHA

if (tableOddRowBackgroundColor == 0) { color = ColorUtils.applyAlpha(paint.getColor(), TABLE_ODD_ROW_DEF_ALPHA); } else { color = tableOddRowBackgroundColor; }

@noties
Copy link
Owner

noties commented Apr 19, 2021

It seems, @ntomasevic-vfy , that you describe a bug that should be fixed 😉 It is better to fix current implementation (to allow transparent color) then putting a temporary plaster of adding an alpha setting (why not adding alpha for other color values?). You intend to use tableOddRowBackgroundColorOpacity with always 0 value, right?

@ntomasevic-vfy
Copy link
Author

Yes, I used it as zero but now I use a workaround (I set any color with opacity 0 but not Color.TRANSPARENT or @android:color/transparent). I was in a hurry and didn't fix the project as it should be. I assume that you should use different condition for checking weather you should use TABLE_ODD_ROW_DEF_ALPHA or not.

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