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

Issue/drawables with tint (Part 5) #9213

Merged
merged 23 commits into from
Feb 12, 2019
Merged

Issue/drawables with tint (Part 5) #9213

merged 23 commits into from
Feb 12, 2019

Conversation

theck13
Copy link
Contributor

@theck13 theck13 commented Feb 11, 2019

Fix

Update drawable usages with tint so that resources can be consolidated and minimized. Since the minSdkVersion is now 21, we can use the android:tint="color" attribute and ImageView.setImageTintList(ColorStateList) method for all drawables. That allows us combine all drawables with the same path into a single file, which removes redundant files and reduces the app footprint. Using tint also helps us with theming and app-wide color changes in the future.

I chose to make white (#ffffff) the default color for drawables since white is the absence of pigment and that correlates to the absence of tint. It makes differentiating tinted and non-tinted icons easier also.

Note

This is the fifth installment of the Drawables with Tint series (Part 1, Part 2, Part 3, Part 4).

Test

Shortcuts

  1. Use device running Nougat (Android 7.1, API 25) or later.
  2. Go to app launcher.
  3. Long-press WordPress app.
  4. Notice icons are shown next to Stats, Notifications, and New post labels.

Activity Log

  1. Select site with ability to rewind activity.
  2. Go to Sites tab.
  3. Tap Activity item.
  4. Tap event able to rewind.
  5. Notice History icon shown next to Rewind label.

Blog Post

  1. Select site with at least one image in a blog post.
  2. Go to Sites tab.
  3. Tap Blog Posts under Publish section.
  4. Tap blog post with at least one image.
  5. Tap image in blog post.
  6. Notice Remove from post icon is shown in top app bar.

Media

  1. Select site with audio in media library.
  2. Go to Sites tab.
  3. Tap Media under Publish section.
  4. Tap Audio tab.
  5. Tap media grid item.
  6. Notice white/black play icon in collapsible toolbar background.
  7. Tap play icon in collapsible toolbar background.
  8. Notice black/white play icon in collapsible toolbar background.

People

  1. Go to Sites tab.
  2. Tap People under Configuration section.
  3. Tap Invite action in top app bar.
  4. Notice Info icon is shown next to Role label.

Site Settings

  1. Select site with Personal, Premium, or Business plan.
  2. Go to Sites tabs.
  3. Tap Settings under Configuration section.
  4. Tap Start Over under Advanced section.
  5. Notice History icon is shown next to Let Us Help label in Start Over screen.

Reader

  1. Use device with slow network connection.
  2. Go to Reader tab.
  3. Notice placeholder with Globe icon is shown for site avatars.
  4. Notice placeholder with User icon is shown for user avatars.

Jetpack Remote Install

  1. Log in with only self-hosted site without Jetpack.
  2. Go to Sites tab.
  3. Tap Stats item.
  4. Tap Install Jetpack button.
  5. Go through Jetpack remote install steps.
  6. Notice Jetpack icon shown throughout installation.

Review

Only one developer is required to review these changes, but anyone can perform the review.

@khaykov khaykov self-assigned this Feb 11, 2019
Copy link
Member

@khaykov khaykov left a comment

Choose a reason for hiding this comment

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

I compared this branch against develop side by side and everything looks good 👍

@khaykov khaykov merged commit f5d7546 into develop Feb 12, 2019
@khaykov khaykov deleted the issue/drawables-with-tint branch February 12, 2019 03:12
@theck13 theck13 restored the issue/drawables-with-tint branch February 12, 2019 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants